Skip to content

Commit 4534793

Browse files
committed
scripts: Always convert result to resolved Promise
As discovered in 18F#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 Test failures from `test/integration-test.js` after upgrading to Node v6.9.1 showed extra output such as: ``` (node:39696) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 2): null (node:39696) PromiseRejectionHandledWarning: Promise rejection was handled asynchronously (rejection id: 2) ``` This was happening because `scripts/slack-github-issues.js` created a Hubot listener that returned a `Promise` so that the integration test could use `.should.be.rejectedWith` assertions. Rather than jump through hoops to quash the warnings, this change now causes the listener's `catch` handler to return the result of the rejected `Promise`, converting it to a fulfilled `Promise` in the process. Since Hubot never used the result anyway, the only effect it has in production is to eliminate the warning messages. The tests now just check that the `Promise` returned by the listener callback is fulfilled with the expected error result, with practically no loss in clarity.
1 parent 1bcfc23 commit 4534793

File tree

2 files changed

+14
-12
lines changed

2 files changed

+14
-12
lines changed

scripts/slack-github-issues.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,13 @@ function fileIssue(filer, response) {
4242
return issueUrl;
4343
})
4444
.catch(function(err) {
45+
var result = err;
46+
4547
if (err) {
46-
response.reply(err.message || err);
48+
result = err.message || err;
49+
response.reply(result);
4750
}
48-
return Promise.reject(err);
51+
return result;
4952
});
5053
}
5154

test/integration-test.js

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ chai.should();
2626
chai.use(chaiAsPromised);
2727

2828
describe('Integration test', function() {
29-
var room, listenerCallbackPromise, logHelper, apiStubServer, config,
29+
var room, listenerResult, logHelper, apiStubServer, config,
3030
apiServerDefaults, reactionAddedMessage, patchReactMethodOntoRoom,
3131
patchListenerCallbackAndImpl, sendReaction, initLogMessages,
3232
wrapInfoMessages, matchingRule = 'reactionName: evergreen_tree, ' +
@@ -158,7 +158,7 @@ describe('Integration test', function() {
158158
});
159159

160160
listener.callback = function(response) {
161-
listenerCallbackPromise = callback(response);
161+
listenerResult = callback(response);
162162
};
163163
};
164164

@@ -179,7 +179,7 @@ describe('Integration test', function() {
179179
sendReaction = function(reactionName) {
180180
logHelper.beginCapture();
181181
return room.user.react('mbland', reactionName)
182-
.then(function() { return listenerCallbackPromise; })
182+
.then(function() { return listenerResult; })
183183
.then(helpers.resolveNextTick, helpers.rejectNextTick)
184184
.then(logHelper.endCaptureResolve(), logHelper.endCaptureReject());
185185
};
@@ -240,8 +240,8 @@ describe('Integration test', function() {
240240

241241
response.statusCode = 500;
242242
response.payload = payload;
243-
return sendReaction(helpers.REACTION)
244-
.should.be.rejectedWith(errorReply).then(function() {
243+
return sendReaction(helpers.REACTION).should.become(errorReply)
244+
.then(function() {
245245
var logMessages;
246246

247247
room.messages.should.eql([
@@ -268,10 +268,9 @@ describe('Integration test', function() {
268268
response.payload = { message: 'should not happen' };
269269
});
270270

271-
return sendReaction('sad-face').should.be.rejectedWith(null)
272-
.then(function() {
273-
room.messages.should.eql([['mbland', 'sad-face']]);
274-
logHelper.filteredMessages().should.eql(initLogMessages());
275-
});
271+
return sendReaction('sad-face').should.become(null).then(function() {
272+
room.messages.should.eql([['mbland', 'sad-face']]);
273+
logHelper.filteredMessages().should.eql(initLogMessages());
274+
});
276275
});
277276
});

0 commit comments

Comments
 (0)