A humble request for an 'uncaughtException' error interceptor

[royaltm]

I have an unusual test case:

1. The callback is being passed to the native binding I'm testing. The callback is being invoked asynchronously from the native code.
2. An error is thrown in the provided callback and I want to make sure that it will end up as the process 'uncaughtException' event and it won't abort the runtime.

Utill tap v9 I just monkey patched t.threw to intercept the error. But starting from v10 it's impossible due to handler being set early and directly and not by a proxy function (e) => this.threw(e).

So my only way now is to remove 'uncaughtException' listeners and fight with domain's uncaughtExceptionHandler - very nasty stuff.

But maybe there is a better way or tap could provide a legal api to test such cases?
From the API perspective I imagine something like:

tap.test('should not abort', function(t) {
  t.plan(3);
  t.throwsUncaughtException(new Error("foo"));
  invoke_native(function(err, done) {
      t.error(err);
      t.ok(done);
      throw Error("foo");
  });
});

and the test will succeed only when the first thrown uncaught exception is Error("foo");

[isaacs]

Ok, so, this is a bit janky, but as of 12.2, you can intercept t.threw in the classic monkeypatch/AOP kind of way like you were doing before v10.

I'm down for a t.throwsUncaughtException(er), but it's quite a bit of a chore to implement, and would be kind of weird. Basically, you'd need to stash the error somewhere, and then in lib/tap.js where it adds the uncaughtException handler, verify that the error matches, and if so... what? You can't continue where you left off, after all.

[royaltm]

Yes, this is weird, but no more weird as testing of asynchronous code in general (without promises). The tested error is being thrown from an asynchronous callback and it terminates its "thread". The obvious limitation is that we can't put more tests behind the thrown error in the asynchronous function. The successful error match in throwsUncaughtException fulfills the plan. If the callback was synchronous we already have t.throws to handle that case.
In the meantime I have produced this crude implementation of throwsUncaughtException for tap 12.0.1, that just works for me.

Probably more legitimate API solution would be if throwsUncaughtException returned a promise that succeeds when the uncaught error is thrown and matches, fails otherwise or doesn't complete when there is no uncaught exception at all. So a continuation dilemma should be solved that way.

Anyway, thanks for the restoration of the ability to just monkeypatch t.threw. This is maybe not the nicest, but very powerful way to extend tap on the run.

[isaacs]

Alright! This totally works with murmurhash-native in the latest v13 release.

const t = require('tap')
const {murmurHash} = require('murmurhash-native')
t.test('should not abort', function(t) {
  t.plan(3)
  t.expectUncaughtException(new Error("foo"))
  murmurHash('hash me!', function(err, done) {
    t.error(err)
    t.ok(done)
    throw Error("foo")
  })
})

You can also expect multiple uncaughtException events, but they must emit in the exact order expected. If any are remaining at the end of the test, then it fails the test (since this happens at test end, technically it fails the parent test, if it's not the root tap object.)

To get it, run npm i tap@next.

[royaltm]

Works with node >= 9.
Unfortunately on node 8 it fails with:

TAP version 13
# Subtest: should not abort
    1..3
    ok 1 - should not error
    ok 2 - expect truthy value
Error: foo
    at /home/royal/node-murmurhash-native/xtest.js:9:11
    not ok 3 - test unfinished
      ---
      stack: |
        Object.<anonymous> (xtest.js:3:3)
      test: should not abort
      at:
        line: 3
        column: 3
        file: xtest.js
        function: Object.<anonymous>
      source: |
        const {murmurHash} = require('.')
        t.test('should not abort', function(t) {
        --^
          t.plan(3)
          t.expectUncaughtException(new Error("foo"))
      ...

    # failed 1 of 3 tests
not ok 1 - should not abort # time=29.727ms

not ok 2 - test end without expected uncaught exceptions # time=29.727ms
  ---
  wanted:
    - - &a1
        message: foo
        name: Error
      - "expect uncaughtException: Error foo"
      - wanted: *a1
  test: should not abort
  ...

1..2
# failed 2 of 2 tests
# time=41.479ms

$ node -v
v8.16.0
$ npm -v
6.4.1
$ npm list tap
└── tap@13.1.8

[isaacs]

Hm, it looks like this is a failing in the ability of async_hooks to track the executionAsyncId across native boundaries in node v8. It's coming back with an eid of 0 instead of 23 (which it should be). This means it can't find the appropriate domain, so it just crashes as if no hook was in place.

I thought it might be feasible to detect this and just assign it to the most recently created domain. Unfortunately, that won't work if you have multiple tests with async behaviors (for example, parallel tests, or just tests that do a bunch of async work before eventually throwing).

There's probably a way to work around this in tap specifically, only for version 8, but I'm not sure. Without the async hook working, it's always going to be dicey to guess, since tests can be running in parallel to one another, and it'll end up being the same kind of brittle approach that you were already using.

At the very least, yes, this is a valid bug (or at least a shortcoming of the design that ought to be documented if it's not going to be fixable). Thanks for following up, and sorry I don't have a better answer :)

[royaltm]

Thanks for looking into it. Looks like I'm stuck with a workaround for a while. At least until I don't need to support node < 9 anymore.

[isaacs]

Yeah, no way to make this work with node v8.

Documented on de3882a as a caveat. Sorry for the inconvenience :\