Skip to content
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

Updates for Node v6.6.0+ UnhandledPromiseRejectionWarning #6

Merged
merged 2 commits into from
Apr 28, 2017

Conversation

mbland
Copy link
Owner

@mbland mbland commented Apr 28, 2017

Backported from mbland/hubot-slack-github-issues#7.

As discovered in 18F/hubot-slack-github-issues#51, the
`UnhandledPromiseRejectionWarning` and `PromiseRejectionHandledWarning`
warnings were apparently added in v6.6.0
(https://nodejs.org/en/blog/release/v6.6.0/); specifically it was added
by nodejs/node#8223. See also:

  nodejs/node#6439
  nodejs/node#8217
  https://nodejs.org/dist/latest-v6.x/docs/api/process.html#process_event_unhandledrejection
  https://nodejs.org/dist/latest-v6.x/docs/api/process.html#process_event_rejectionhandled
  https://nodejs.org/dist/latest-v6.x/docs/api/process.html#process_event_warning

A test failure from `solutions/06-integration/test/integration-test.js`
after upgrading to Node v6.9.1 showed output such as:

```
-  "(node:87412) UnhandledPromiseRejectionWarning: Unhandled
     promise rejection (rejection id: 14): Error: failed to create a
     GitHub issue in 18F/handbook: received 500 response from GitHub
     API: {\"message\":\"test failure\"}\n"
```

This was happening because `scripts/slack-github-issues.js`
ignored the return value from `Middleware.execute()`, whether it was
undefined or a `Promise`. For consistency's sake (and to provide a
clearer upgrade path to the current state of
mbland/slack-github-issues), `Middleware.execute()` always returns a
`Promise`, and `scripts/slack-github-issues.js` handles and ignores any
rejected Promises.

This fixed the `integration-test.js` error, but also required minor
updates to `solutions/{05-middleware,complete}/test/middleware-test.js`.
The next commit will update the tutorial narrative to account for this
change.
@mbland mbland added the bug label Apr 28, 2017
@mbland mbland self-assigned this Apr 28, 2017
@mbland mbland merged commit 22c3df2 into master Apr 28, 2017
@mbland mbland deleted the unhandled-promises branch April 28, 2017 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant