New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect uncaught rejection in Promise #1926

Open
kai670 opened this Issue Oct 16, 2015 · 5 comments

Comments

Projects
None yet
6 participants
@kai670

kai670 commented Oct 16, 2015

If I trigger an event from a promise onFulfilled function and an error is thrown from the handler of the event mocha doesn't detect this Error.

I create a fiddle where this can be seen: http://jsfiddle.net/dsrhmvLz/3/.

AFAIK there is no standard way of detecting this unhandled promise rejection but using bluebird it can be detected with the unhandledrejection event as seen in the fiddle. Could it make sense adding support for this in mocha? There is another way of detecting this? If I'm not wrong currently something like this is done in mocha with the uncaughtException event.

With this little change (http://jsfiddle.net/dsrhmvLz/4/) the error is detected by mocha but it would seem nicer seeing the uncaught rejection in mocha without that trouble.

@danielstjules

This comment has been minimized.

Show comment
Hide comment
@danielstjules

danielstjules Oct 16, 2015

Contributor

We could take advantage of https://nodejs.org/api/process.html#process_event_unhandledrejection in newer versions of node as well

Contributor

danielstjules commented Oct 16, 2015

We could take advantage of https://nodejs.org/api/process.html#process_event_unhandledrejection in newer versions of node as well

@kevinoid

This comment has been minimized.

Show comment
Hide comment
@kevinoid

kevinoid Feb 6, 2016

I'd like to see support for tracking unhandledRejection included as well. For reference, I'm currently converting any unhandledRejection into unhandledException in a global helper using the following code. This seems to work quite well for me.

if (typeof process !== 'undefined') {
  process.on('unhandledRejection', function (reason) {
    throw reason;
  });
} else if (typeof window !== 'undefined') {
  // 2016-02-01: No browsers support this natively, however bluebird, when.js,
  // and probably other libraries do.
  if (typeof window.addEventListener === 'function') {
    window.addEventListener('unhandledrejection', function (evt) {
      throw evt.detail.reason;
    });
  } else {
    var oldOHR = window.onunhandledrejection;
    window.onunhandledrejection = function (evt) {
      if (typeof oldOHR === 'function') oldOHR.apply(this, arguments);
      throw evt.detail.reason;
    };
  }
} else if (typeof console !== 'undefined' &&
    typeof (console.error || console.log) === 'function') {
  (console.error || console.log)('Unhandled rejections will be ignored!');
}

kevinoid commented Feb 6, 2016

I'd like to see support for tracking unhandledRejection included as well. For reference, I'm currently converting any unhandledRejection into unhandledException in a global helper using the following code. This seems to work quite well for me.

if (typeof process !== 'undefined') {
  process.on('unhandledRejection', function (reason) {
    throw reason;
  });
} else if (typeof window !== 'undefined') {
  // 2016-02-01: No browsers support this natively, however bluebird, when.js,
  // and probably other libraries do.
  if (typeof window.addEventListener === 'function') {
    window.addEventListener('unhandledrejection', function (evt) {
      throw evt.detail.reason;
    });
  } else {
    var oldOHR = window.onunhandledrejection;
    window.onunhandledrejection = function (evt) {
      if (typeof oldOHR === 'function') oldOHR.apply(this, arguments);
      throw evt.detail.reason;
    };
  }
} else if (typeof console !== 'undefined' &&
    typeof (console.error || console.log) === 'function') {
  (console.error || console.log)('Unhandled rejections will be ignored!');
}
@voxpelli

This comment has been minimized.

Show comment
Hide comment
@voxpelli

voxpelli Jul 21, 2016

I agree that it would be great to have this support, especially in addition to the timeout message. Today when an error unintentionally gets caught in a Promise, eg. because an error was thrown from within callback that wasn't expected to be called from a Promise chain, one just gets a timeout error and no indication on what the cause may be.

Adding any unhandled rejection data to at least the timeout error would be very helpful:

  1) Foo should bar:
     Error: timeout of 2000ms exceeded. Ensure the done() callback is being called in this test.

(The possibility of generally considering unhandled rejections to be test failures would also be kind of nice)

voxpelli commented Jul 21, 2016

I agree that it would be great to have this support, especially in addition to the timeout message. Today when an error unintentionally gets caught in a Promise, eg. because an error was thrown from within callback that wasn't expected to be called from a Promise chain, one just gets a timeout error and no indication on what the cause may be.

Adding any unhandled rejection data to at least the timeout error would be very helpful:

  1) Foo should bar:
     Error: timeout of 2000ms exceeded. Ensure the done() callback is being called in this test.

(The possibility of generally considering unhandled rejections to be test failures would also be kind of nice)

@boneskull

This comment has been minimized.

Show comment
Hide comment
@boneskull

boneskull Jul 21, 2016

Member

@voxpelli I've written some stuff around the capability to detect and properly handle uncaught async exceptions here, but probably haven't covered this use-case.

Originally, I had wanted to even detect async execution via async_wrap, but depending on the version of Node.js used, even a console.log() will make an async write to the stdout stream...

Member

boneskull commented Jul 21, 2016

@voxpelli I've written some stuff around the capability to detect and properly handle uncaught async exceptions here, but probably haven't covered this use-case.

Originally, I had wanted to even detect async execution via async_wrap, but depending on the version of Node.js used, even a console.log() will make an async write to the stdout stream...

@ScottFreeCode

This comment has been minimized.

Show comment
Hide comment
@ScottFreeCode

ScottFreeCode Sep 8, 2017

Member

Are this and #2640 duplicates of each other?

Member

ScottFreeCode commented Sep 8, 2017

Are this and #2640 duplicates of each other?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment