hubot-archive / hubot-pager-me

PagerDuty integration for Hubot
https://www.npmjs.com/package/hubot-pager-me
MIT License
73 stars 92 forks source link

No response given for invalid issue numbers #105

Open ghost opened 7 years ago

ghost commented 7 years ago

When triggered with an invalid/non existent issue number while adding notes eg pager note 999999 chicken on fire Hubot does not respond in the chat/channel to say there's been an issue adding the note.

While there's no response given by hubot in the channel an http 400 error message is seen in the hubot logs:

ERROR Error: 400 back from /incidents/999999/notes
    at PagerDutyError.Error (native)
    at new PagerDutyError (/home/hubot/node_modules/hubot-pager-me/src/pagerduty.coffee:10:7)
    at /home/hubot/node_modules/hubot-pager-me/src/pagerduty.coffee:86:23
    at IncomingMessage.<anonymous> (/home/hubot/node_modules/hubot-pager-me/node_modules/scoped-http-client/src/index.js:95:22)
    at emitNone (events.js:72:20)
    at IncomingMessage.emit (events.js:166:7)
    at endReadableNT (_stream_readable.js:923:12)
    at nextTickCallbackWith2Args (node.js:511:9)
    at process._tickCallback (node.js:425:17)
stephenyeargin commented 7 years ago

My guess is that the return from err allows the bot to emit the error, but doesn't provide user feedback. As this pattern happens elsewhere in the code, it might not be isolated to notes.

https://github.com/hubot-scripts/hubot-pager-me/blob/c89cec0203033b8382db900bc021350f2f1bad48/src/scripts/pagerduty.coffee#L292-L300

ghost commented 7 years ago

Looks like notes, overrides and maintenance_windows may also be affected as the use they pagerduty.post function.

The post function in the pagerduty module is expecting a 201 response from PD:

https://github.com/hubot-scripts/hubot-pager-me/blob/master/src/pagerduty.coffee#L83-L87

whats best to do here; fix it now or wait for the API V2 stuff to come through ?

stephenyeargin commented 7 years ago

whats best to do here; fix it now or wait for the API V2 stuff to come through ?

I actually don't have a PagerDuty login anymore, so I personally can't develop against it. Whatever V2 work would probably encounter the same issue unless it was refactored entirely.