test: improve async-hooks/test-callback-error

[refack]

Ref: #13527

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• commit message follows commit guidelines

Affected core subsystem(s)

test

[trevnorris]

Will this conflict w/ #13554 ?

[refack]

Will this conflict w/ #13554 ?

Somewhat, but we're both trying to stabilize it.

[refack]

Stressing macOS: https://ci.nodejs.org/job/node-stress-single-test/1281/nodes=osx1010/

[Trott]

@refack On the stress test, you might want to include --timeout 60 or even --timeout 30 or less. If the problem is that the test will run but just take longer than 60 seconds to complete sometimes for whatever reason, the stress test will come up clean if you don't specify a timeout (I think).

[refack]

@refack On the stress test, you might want to include --timeout 60 or even --timeout 30 or less. If the problem is that the test will run but just take longer than 60 seconds to complete sometimes for whatever reason, the stress test will come up clean if you don't specify a timeout (I think).

From my manual inspection if all's well it should complete quite quick

[refack]

Restressing with --timeout 30 (and timestamps): https://ci.nodejs.org/job/node-stress-single-test/1282/nodes=osx1010/

[AndreasMadsen]

If you are working on this then please also add the if (process.argv[2]) wrapper

if (process.argv[2]) {
  // child
} else {
  // parent with assert
}

such we don't depend on the test to work (child must throw) for it not to fork recursively.

[refack]

If you are working on this then please also add the if (process.argv[2]) wrapper

Added returns
Also the stdout/stderr parsing was Windows incompatible

[refack]

Stress: https://ci.nodejs.org/job/node-stress-single-test/1284/nodes=osx1010/
https://ci.nodejs.org/job/node-stress-single-test/1286/nodes=osx1010/
Full CI: https://ci.nodejs.org/job/node-test-pull-request/8576/

[refack]

Stress my [try]: https://ci.nodejs.org/job/node-stress-single-test/1287/nodes=osx1010/

[AndreasMadsen]

The only relation between --abort-on-uncaught-exception and async_hooks I can think of is src/env-inl.h#L155L159.

Comparing with process.abort() I don't know why fprintf(stderr, "\n"); fflush(stderr); is needed, that being said I also don't see any problem with it.

[Trott]

/cc @DavidCai1993

[refack]

Stress macOS: https://ci.nodejs.org/job/node-stress-single-test/nodes=osx1010/1288/

[refack]

Stress passed. PTAL.
Full CI: https://ci.nodejs.org/job/node-test-pull-request/8581/
(andother more "tight" stress - https://ci.nodejs.org/job/node-stress-single-test/nodes=osx1010/1289/)

[refack]

@Trott something changed. CI stopped repoducing

[refack]

Rebaed on older dde4f0f
https://ci.nodejs.org/job/node-stress-single-test/1293/nodes=osx1010/

[refack]

@rvagg is it possible that the outage (nodejs/build#749) fixed this?
(tl;dr process.abort() on the macOS CI machine took > 20s about two days ago now it's ~100ms)

[AndreasMadsen]

Can you run the normal CI on all platforms?

CI: https://ci.nodejs.org/job/node-test-pull-request/8727/

requested changes done, dismissing review

[refack]

Gate: https://ci.nodejs.org/job/node-test-commit-linuxone/6725/ ✔️
Full: https://ci.nodejs.org/job/node-test-pull-request/8728/

[refack]

So all but one machines passed SIGABRT
ubuntu1604 passed SIGSEGV

2	async-hooks/test-callback-error	
duration_ms	0.761
severity	fail
stack	|-
assert.js:60
  throw new errors.AssertionError({
  ^

AssertionError [ERR_ASSERTION]: 'SIGSEGV' === 'SIGABRT'
    at ChildProcess.child.on (/home/iojs/build/workspace/node-test-commit-linux/nodes/ubuntu1604_docker_alpine34-64/test/async-hooks/test-callback-error.js:98:14)
    at emitTwo (events.js:125:13)
    at ChildProcess.emit (events.js:213:7)
    at maybeClose (internal/child_process.js:898:16)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:209:5)

Is this a bug or an acceptable value?
@gireeshpunathil

[refack]

Rerunning linux to see if SIGSEGV is consistent: https://ci.nodejs.org/job/node-test-commit-linux/10708/

[refack]

So it's consistent but it's docker_alpine34 Ubuntu is just the host

[refack]

Added bail clause if alpine acts strange.
https://ci.nodejs.org/job/node-test-commit/10691/
I'm gonna land this since test-requireio-osx1010-x64-1 is in a bad mood again.

[refack]

Once more on linux: https://ci.nodejs.org/job/node-test-commit-linux/10712/

[refack]

Extra sanity of master: https://ci.nodejs.org/job/node-test-commit-linuxone/6785/
Extra CI on macOS: https://ci.nodejs.org/job/node-test-commit-osx/10641/

[Trott]

Seems like the test is now failing every single time rather than intermittently. For me at least, make -j4 test on local macOS machines now always fail with this change. I think we might want to undo this until we get that sorted out? make -j4 test failing consistently means we have a broken build IMO.

@nodejs/platform-macos

[Trott]

Repeating what I said in another issue: Just to be clear: No shame intended! A lot of great work has happened so far and some really important information has been uncovered. These things happen.

[refack]

Repeating what I said in another issue: Just to be clear: No shame intended! A lot of great work has happened so far and some really important information has been uncovered. These things happen.

Most of the work that gone into this PR was to allow for a more informative fails...

[gireeshpunathil]

@refack - sorry, I missed my mention in the PR. SIGSEGV seems to be unexpected, and I have seen this in AIX as well which is identified as a memory corruption. Working on some plans to track the offending code at the moment. Where should we track the Linux failure?

[refack]

@refack - sorry, I missed my mention in the PR. SIGSEGV seems to be unexpected, and I have seen this in AIX as well which is identified as a memory corruption. Working on some plans to track the offending code at the moment. Where should we track the Linux failure?

I've opened #13865. commented that maybe that how musl libc implements abort.

[asfourco]

ping @jasnell