fixing KeyError for X-Request-Id header, use X-Github-Delivery for event ID instead #2
Conversation
…med to X-Github-Delivery
| @@ -27,7 +27,7 @@ def get_event_info(request): | |||
| """ | |||
| event_info = { | |||
| 'action': request.json.get('action', UNKNOWN), | |||
| 'id': request.headers['X-Request-Id'], | |||
| 'id': request.headers['X-Github-Delivery'], | |||
There was a problem hiding this comment.
I'm a bit puzzled by this, and X-Request-Id suddenly isn't there anymore...
Maybe that's my mistake, and I'm relying on something that's not always there in the header.
Where did you get the X-Github-Delivery from, is there a reference for that, that this uniquely identifies an event?
I'm wondering whether we should use this instead:
'id': request.headers.get('X-Request-Id') or request.headers.get('X-Github-Delivery'),Maybe we also need to make sure that we get proper error reporting if neither of these keys are available?
There was a problem hiding this comment.
Based on https://apievangelist.com/2017/09/13/webhook-delivery-headers-from-github-api/, X-GitHub-Delivery indeed seems to be the correct field to use to get a unique ID for the event
There was a problem hiding this comment.
I logged the HTTP headers and compared their values with the "Event ID" shown on the smee.io/channel page. I don't know if there is a reference.
Apparently, both headers may be valid, see octokit/webhooks.js#579 Don't know if they can be present both in a single request. Your suggestion to use one of the headers might be the right way to go forward.
There was a problem hiding this comment.
Possibly, the X-Request-Id was not present in my limited tests only triggering installation events.
|
@trz42 Tests are failing because there it's still assumed that only providing |
boegel
changed the title
fixing KeyError for non-existing X-Request-Id header; apparently X-Request-ID was renamed to X-Github-Delivery