Add handleRejections support

[drazisil]

Adds #921

However, needs some work because, although this seems like it should work, I don't think the tests are right since no log is created in fixtures.

It almost feels like it's silently not working, but I can't figure out how to get past mocha's "make sure your promise resolves" in order to test that it rejects. (and handled by winston)

Hope this is a good starting point for someone. Leaving the WIP tag to indicate it shouldn't be merged without double-checking the testing.

[DABH]

Thanks for this PR! Looks very nice! 😎 Folks have wanted this feature for a while.

Re: tests, do you have e.g. a plain node.js file you or I can run to see if this works or not (i.e., outside of mocha or any other test frameworks)? I can take a look at the mocha stuff but it would be helpful to have some starting point for testing. Thanks again for your work on this!

[drazisil]

@DABH The below code will throw an unhandled rejection:

var promise1 = new Promise(function(resolve, reject) {
  setTimeout(function() {
    reject('foo')
  }, 100)
})

(node:19646) UnhandledPromiseRejectionWarning: foo
(node:19646) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). (rejection id: 1)
(node:19646) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

Under Node 8.11. I tried figuring out a way to test it but didn't have much success.

[drazisil]

Updated package-lock.json so there are no conflicts. Since the changes are approved, will this be merged soon? Not sure if anyone has looked into the testing. cc: @DABH

[avik-so]

Hi, is this fix safe? Our team is relying on it but was waiting for it to be officially merged into the project.

[awill1988]

Please merge!

[awill1988]

Line 87 in lib/winston/rejection-handler.js has a typo, unhandledJejection

[drazisil]

Corrected, @awill1988

[indexzero]

Howdy folks – life happened for a little while over here. @DABH and I have monthly syncs to review & triage large features like this, but I missed the last two 😳

As such we'll be doing some overdue (my fault) maintenance in the next week or so. Keep an eye out for a minor release.

In principle winston needs to handle uncaughtRejection – appreciate someone from the community digging in and taking a stab implementing it.

[kingnebby]

@indexzero - may I just say thank you for being awesome. No need to apologize for that.

[DABH]

Since tests look like they're passing now, I think that this should be able to drop in the next release. I looked through the code again and it still looks good. The one thing I'm concerned about is exit behavior -- with ExceptionHandler, there are several open issues right now about e.g. exceptions not being logged before the program exits (I have a PR to fix that though). But I wonder if there's a similar issue with rejections since code is mostly same as ExceptionHandler. In any case we could probably look into that after merging this, especially if people are already using this feature branch and finding it useful/working.

[ejmartin504]

FWIW I would love to see this merged / released as well, and I'd be happy to help out with any outstanding issues that give you pause @DABH :-)

[indexzero]

Reviewed this with @DABH today and we are 👍 – David is going to apply his fix from #1355 to this post-merge and then it will go out in 3.2.0. See the remaining issues in the release.

[glensc]

Adds missing typescript typings: #1842