Fail with error & stack trace in asynchronous code

[stevecj]

When an error is thrown from asynchronous code, Jasmine currently has no way of knowing that it happened, skipping subsequent stages (e.g. "it" following error in "beforeEach") and reporting a trace.

As a workaround, I have written helper code to catch and record the error from a function execution so that it can be re-thrown from an "afterEach" block. This solution at least gives a failure and a stack trace, but still leaves a lot to be desired.

For my partial workaround, see the "asyncStep" and "getCaptureAsyncError" functions in https://github.com/stevecj/headway/blob/master/spec/support/userContextExt.js . To see how those are used, see https://github.com/stevecj/headway/blob/master/spec/indexedDbAdapterSpec.js .

[sheelc]

Handling asynchronous errors is something that we would like to look at in future releases in Jasmine.

Thanks for sharing your implementation. Unfortunately since Jasmine won't be able to wrap the asynchronous code in a try/catch we'll probably have to end up relying on window.onerror/process.on('uncaughtException'). We've added a story to the backlog to explore this issue here, but happy to look at pull requests as well.

[stevecj]

Correction to above. Since I moved files around, see https://github.com/stevecj/headway/blob/master/spec/indexedDbAdapter/coreSpec.js for example of how the helpers are used.

The way I'm applying the try/catch is to have the programmer pass a function to this.asyncStep(...) which then returns a wrapped function. The resulting wrapped function can then be supplied to whatever will make the asynchronous call.

[sheelc]

Ah interesting. Seems like that would work for callback parts of the code in the spec but still not parts of implementation code that could throw? For example in the following code taken from coreSpec.js:

beforeEach( function ( done ) {
  this.getSubject().asyncConnect( DB_NAME ).then(
    this.asyncStep( function ( db ) {
      db.close();
      // Only care about calls on re-open, not previous open to create db.
      this.getSchema().migrate.calls.reset();
    })
  ).then( done, done );
});

If asyncConnect was implemented like the following:

asyncConnect = function(dbName) {
  var promise = new Promise();
  setTimeout(1, function() { 
    dataBaseConnector.connect(dbName).then(function(db) {
      promise.resolve(db); 
    });
  });

  return promise;
};

This implementation is a bit contrived, but if dataBaseConnector.connect raises an exception, it doesn't seem like asyncStep would catch it. To make this case work without window.error would require wrapping implementation code with asyncStep-like functionality -- which seems not ideal.

[stevecj]

I see what you're saying. In my case, I have taken responsibility (as the developer) for making sure that the code that is likely to throw exceptions can be wrapped, and the only thing I'm relying on the helper to do is to notice the exception and deal with it. The only thing that I've been unhappy about with my helper is that it can't halt the example run when an exception is detected.

Of course, it would be nicer if this was magically handled without having to wrap anything, but I'm not sure if that's possible to do in such a way that we can know the error belongs to the current example and not to a previous example that kept running after timeout (happens a lot with mistakes using indexedDB that cause blocking) or to some setInterval handler that never got cleared.

[sheelc]

@stevecj That's a great point. Simply getting an error doesn't mean that we should fail the current spec.

Seems like at least currently, minus wrongly assuming the current spec, it would have to be the developers that deal with exceptions that happen in their async code. I'm not really sure how we can proceed.

[stevecj]

IMO, a good step forward would be something similar to the helper I wrote, but that can also halt the example in its currently in-progress step or output the full example name that it belongs to if already timed out.

I just had this thought — maybe dumb, or maybe clever. What if the "done" argument could double as as an object with member functions such as the one I'm suggesting for wrapping async substeps (rather than adding a method to "this" as I have been doing)? Perhaps, there could also be a prescribed technique for adding other custom support methods to "done".

If this idea sounds good to you, I wouldn't mind attempting to work up a PR.

[infews]

If this is still interesting to you, we'd be happy to look at a pull request. I need much more context in order to consider it.

[stevecj]

No time in mys schedule at the moment, but definitely still interesting. I will try to make a PR sometime in the nearish future.

[slackersoft]

We have this tracker story (https://www.pivotaltracker.com/story/show/66359332) as well

[mik01aj]

I don't know if this is the same or different issue, but I discovered that when I do this somewhere:

setTimeout(function () {
    throw 'kaboom';
}, 1);

then what I get is:

PhantomJS 1.9.8 (Mac OS X) ERROR
  kaboom
  at undefined

Finished in 0.002 secs / 0.014 secs

SUMMARY:
✔ 16 tests completed

So the reporter thinks that my tests pass... but it actually ran only 16 out of 52 tests I currently have. 😱

[slackersoft]

The fact that the exception does not actually cause the suite to fail is the problem here. I'm not sure why it didn't run all of your tests, and am actually unable to reproduce that issue.

[mik01aj]

One more similar issue: when I do:

    setTimeout(function() {
        expect(true).toBe(false);
    }, 10);

...it fails some random async spec later.

[slackersoft]

@mik01aj that actually doesn't surprise me. Once you get into setTimeout there isn't really any way to trace the failure back to any particular spec. So, if your spec doesn't have a done callback that it invokes at the end of the setTimeout function, jasmine will just associate the expectation with whichever spec is actually being executed when the timeout elapses and the function is invoked.

That is probably not something we're going to worry about, since it's failing the suite and you've basically set it up so jasmine can't figure out where the correct place to put the failure is.

[adrianolek]

The issue is when a spec does have a done callback and the async code throws some exception before executing done().
Something like this:

describe('suite', function () {
  it('spec 1', function (done) {
    // some async code that may eventually throw an exception
    setTimeout(function () {
      throw 'foo';
      done();
    });
  });
  it('spec 2', function () {
    expect(true).toBeTruthy();
  });
});

The runner will crash while executing spec 1. The remaining specs (spec 2) won't run at all.

[slackersoft]

@adrianolek Simply throwing an error in the background shouldn't be causing the whole runner to crash. What I would expect to happen right now is, throw causes the done never to get called and the QueueRunner then times out the execution of that step and moves on.

If you are seeing the entire suite stop upon encountering an error in async code, that definitely sounds like a bug. What version of jasmine are you using? Can you reproduce this issue with a small suite of just the two specs?

Thanks

[adrianolek]

@slackersoft here it is. I've just noticed jasmine/jasmine is the browser runner and I'm using jasmine/jasmine-npm. However the issue seems to be the same. I could add an issue in jasmine-npm if you prefer.

[slackersoft]

@adrianolek it sounds like the crash is particular to running the specs in node, since we don't yet have a global exception handler while running specs. We want to get this fixed soon, especially now that I more fully understand how it is affecting the npm runner, but I'm not sure when we'll get some time to look at how best to accomplish this.

I would be happy to review a pull request that caught exceptions from async code in both a browser and nodejs context and got that failure to the current runnable to cause the failure. Also see the tracker story for more info.

[attila123]

@slackersoft thanks for this commit.
Is there any special configuration to make this work?
I updated jasmine-core to be 2.6.1 in my package.json + npm update-ed it, and I double checked that this change got into jasmine.js.
Still the error which is thrown inside a callback function in a code in a beforeAll is not printed at all.

  beforeAll(done => {
    doSomething('blah', result => {
      // inside the callback function
      // business logic...
      throw new Error('An error happens here');
      // business logic...
      // done would be called at the end
      done();
    });
  });

I use jasmine via protractor.
With a simple Promise based example I could make Jasmine to print the error with a stack trace:

  beforeAll(done => {
    new Promise((resolve, reject) => {
      reject('emulate an error');
    }).then(done).catch(err => { done.fail(err); } );
  });

Would it be the right pattern to follow, e.g. always do catch-ing and call done.fail in it like in the above example?
If I remove the catch-ing (remove '.catch(err => { done.fail(err); } )') from the code above, then I just get the following line:

(node:5561) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 2): emulate an error

and test execution stucks for quite some time as obviously done or done.fail does not get called.

The above pattern seems to work great also if an Error is thrown (error and stack trace are logged):

  beforeAll(done => {
    new Promise((resolve, reject) => {
      throw new Error('error is thrown here');
    }).then(done).catch(err => { done.fail(err); } );
  });

A bit more complex example, which also works nice (provided that the catch "boilerplate" is used at the end):

  beforeAll(done => {
    let asyncFunc1 = () => {
      return new Promise((resolve, reject) => {
        resolve('some long running task finishes successfully');
      });
    };

    let asyncFunc2 = (arg) => {
      return new Promise((resolve, reject) => {
        console.log('arg', arg);
        reject('The second one fails');
      });
    };

    asyncFunc1().then(res => asyncFunc2(res)).then(done).catch(err => { done.fail(err); });
  });

[attila123]

I checked this again with a colleague. It seems to depend on the Node.js version!!!
If I throw an Error from a plain old callback function inside beforeAll:

• it is correctly logged running with Node.js v6.9.5
• it is not logged at all with Node.js v6.10.3
  (currently I have these two versions of Node.js installed with nvm).

[slackersoft]

@attila123 Jasmine just uses the uncaughtException event on process to setup a handler, so if this context changed between the two versions, that could break it.

Another thing to check is that the Jasmine being run in the different versions is really 2.6.x and not accidentally 2.5 or something.

When the error isn't logged, what other behavior are you seeing? Does your suite still pass? Does the spec timeout?, etc.

[lathonez]

I've been testing this today and was hoping someone could confirm the intended behaviour.

The following (as of 2.6.1) will fail my test suite:

it('fails on an uncaught async error', () => {

  // should fail somewhere because of this?
  setTimeout(
    (() => {
      throw 'kaboom';
    }),
    1
  );
});

But the same thing in a Promise will not:

  it('fails on an uncaught error thrown in a promise', () => {

    let prom: Promise<{}> = new Promise((resolve, reject) => {
      resolve();
    });

    prom.then(() => {
      throw 'kaboom';
    });
  });

In the above test - I get the following in my console but all the tests pass. Is this expected?

zone.js:571 Error: Uncaught (in promise): kaboom
    at ProxyZoneSpec.Array.concat.ProxyZoneSpec.onInvokeTask (http://localhost:9876/base/src/test.ts:164313:39) [ProxyZone]
    at <anonymous> [<root>]
consoleError @ zone.js:571

Thanks

[attila123]

@slackersoft I double checked that I was on the latest jasmine-core 2.6.1. I checked this fix of yours appeared in node_modules/jasmine-core/lib/jasmine-core/jasmine.js.
Because there was an error thrown in the async callback function, my code never managed to call done(), and therefore it stuck in beforeAll for the 10 minutes (I think that is the default timeout). After that I think it timed out, but and I am not whether it-s failed or not, and don't have time to experience with that.
With nvm you can install multiple versions of Node.js easily, and you can change between them easily, e.g. 'nvm use v6.9.5' (changes version temporarily in current shell), or 'nvm alias default v6.10.3' (changes version permanently, so when you open a new terminal, node will be the at the desired version).
Hope this helps, and you can reproduce it.

[sgravrock]

@lathonez: Both of your example tests tell Jasmine that they've passed (in this case by returning without failing) and then later trigger an exception. Jasmine tries to associate that kind of late asynchronous error with the spec that most likely caused it, but it's not an easy problem to solve. #1352 might help in your scenario by routing the errors somewhere useful, but it's also possible that the errors are occurring after Jasmine has reported success.

It doesn't surprise me that the promise test behaves differently than the setTimeout tests. Most promise implementations use a more complicated stack-clearing scheme than a simple setTimeout, and that can affect the timing of the error.

[lathonez]

@sgravrock - thank you for getting back to me

Both of your example tests tell Jasmine that they've passed (in this case by returning without failing)

We had a test that was fully synchronous, but the implementation changed to be async and the dev didn't update the spec correctly to match. It took me a few weeks to realise the async part of the implementation was failing in Jasmine. The usecase is as per the example - if the code were synchronous the tests would have failed as the implementation threw an error.

Jasmine tries to associate that kind of late asynchronous error with the spec that most likely caused it, but it's not an easy problem to solve.

I understand - I don't care whether or not the failure is associated with the correct spec - just failing the entire test suite on an uncaught error is good enough for me. This is the behaviour I observed on 2.6.1 with the setTimeout example. All the tests ran until the end and then the suite failed.

#1352 might help in your scenario by routing the errors somewhere useful,

Thank you, will keep an eye on this

but it's also possible that the errors are occurring after Jasmine has reported success.

Not possible in the sense that they are observed in the console long before the suite finishes running. But I guess perhaps before Jasmine has had them bubble up or something?

It doesn't surprise me that the promise test behaves differently than the setTimeout tests.

Agreed. I was trying to understand whether the promise example I gave should have caused Jamine to fail as of 2.6.1. I'm still not certain whether it should have or not?

[slackersoft]

Not possible in the sense that they are observed in the console long before the suite finishes running. But I guess perhaps before Jasmine has had them bubble up or something?

Jasmine finishes running as soon as the last dot is printed, but the full report might take some time to generate in larger suites.

I'm releasing 2.6.2 tonight, give that a shot with some of the other fixes, hopefully it will help. Thanks for using Jasmine!

[lathonez]

FYI I've tested on 2.6.2 and have the same behaviour re promises.

This is purely for information - I didn't come here expecting a fix for this, just thought it might fix it and wanted to help document behaviour.

If others have different results I would be keen to know.

@slackersoft thanks for all your work, very happy with Jasmine.

[attila123]

Hi, I'd like to test a bit jasmine with newer (6.10, 6.11) Node.js. Does anyone know a sample project to add simple jasmine tests to? So that I could simply share some example tests based on that project.
(My problem is that for testing our app we need to use Node.js 6.9.5 (which works correctly with jasmine) for most tests, e.g. unit tests, but we release the app on an image with 6.11.1 (which has the latest security fixes), and so only during the end-to-end tests our app runs on 6.11.1 (but still the protractor, jasmine runs on 6.9.5).)

[slackersoft]

@attila123 You are more likely to get quicker responses from the community for "How to use jasmine?" questions and a history of other solutions on the jasmine-js group. We try to keep jasmine's github issues list focused on bugs and feature requests for jasmine itself.

Additionally, your comment has nothing to do with the issue to which you are replying. If you have an issue to report, please create a new issue. Thanks for using Jasmine!

[dpmott]

This testing approach might provide a short-term or case-by-case alternative to changing the Jasmine framework:
https://stackoverflow.com/questions/46006944/how-to-expect-an-asynchronously-thrown-exception-in-jasmine-angular2-typescr

[applecool]

@slackersoft With the latest native async support in Jasmine, I can simply write an async test like follows:

beforeEach(async () => {
  await browser.get(theUri);
});

which is pretty cool.

But, in this case, we are not handling any errors.. How, can I do error handling here so that I can log my error messages to the console which would help me in debugging? Could you give me any pointers?

[slackersoft]

If you use the async keyword or return a Promise, Jasmine should handle a rejected promise as a failing spec and report the error that way. Additionally, Jasmine should also be catching any globally unhandled errors and associating them to the currently executing spec as well. If you are not seeing this happen, please open a new issue with more details.

Thanks for using Jasmine!

[applecool]

@slackersoft True, I am actually looking for a way to use console.error() in chrome which gives me the specific line and file name in the console when a particular test fails.

Before the native async support, I used to use my own custom done() function which takes in async function as an argument and processes it:

function done(asyncFn: () => Promise<any>) {
    return function (done) {
        asyncFn().then(done)
            .catch(e => {
                console.error(e); // This line gives me the error in Chrome console with TS file line.
                done.fail(e);
            })
            .catch(() => {
                done();
            });
    }
}

Now, I removed this done() function, and simply use async in my test specs like follows:

it('test async', async() => {
   fixture.detectChanges();
   await fixture.whenStable(); // Angular test bed framework.
   expect(true).toBe(true);
});

The same spec used to look like follows with my custom done():

it('test async', done(async() => {
       fixture.detectChanges();
       await fixture.whenStable(); // Angular test bed framework.
       expect(true).toBe(true);
    }));

[slackersoft]

@applecool if you would like to see more/different information reported when a rejected promise causes a spec to fail, please open a new issue with the additional information you'd like to see. A rejection that comes through with an Error object should already be reporting its message and stack so if you're not seeing this information, we would like to remedy this.

Thanks for using Jasmine!

[applecool]

@slackersoft I will open an issue. Thank you. I was wondering if you could just point me in a right direction here: I am trying to override the it() function and want to handle the errors in there if the argument passed to it is an async function. Here's what I have got so far, but I am unable to check programmatically if the function is async or not. It would be really helpful if you could suggest me some alternative:

    const asyncFn = async () => {};
    let originalIt = jasmine.getEnv().it;
    jasmine.getEnv().it = function(...args): jasmine.Spec {
        if(arguments[1].constructor === asyncFn.constructor) { 
              // This isn't working. It resolves to true for all the functions.
                //  Attempt 1: Do error handling.. 
        }
       if(arguments[1] instanceof Promise) {
            // This isn't working either. It never resolves to true.
           // Attempt 2: Do error handling
       }
        return originalIt.apply(this, arguments);
    }

I also wanted to know if I am on the correct path of overwriting the it() function. I have never done monkey patching before but this is an attempt to do so and try to learn from it. Looking forward to your suggestions.

Thank you.

[dmurvihill]

Thanks for fixing this! Is there a matcher for these async errors?

[sgravrock]

Thanks for fixing this! Is there a matcher for these async errors?

Yes and no. It depends on what you're trying to accomplish. If you're trying to verify that your async code reports errors by rejecting a promise and doesn't trigger a global error, then you might use expectAsync(...).toBeRejected() or expectAsync(...).toBeRejectedWithError(...) in combination with letting Jasmine report a failure if a global error occurs.

On the other hand, if you want your test to pass when a global error occurs and fail when it doesn't, then there's nothing built-in for that. I've seen some people spy on window.onerror or the equivalent Node interface and expect it to have been called with certain arguments.