Error event should include response object

[bahamat]

When error is emitted, the res object has already been fulfilled and ended. The error event should include the res object so that it can be included in logs, or otherwise acted upon.

[rjz]

@bahamat I notice that our response body will just be the error, wrapped in reasonable HTTP semantics):

res.writeHead(400, { 'content-type': 'application/json' })
res.end(JSON.stringify({ error: msg }))

var err = new Error(msg)
// ...

Rather than explicitly emitting res, could downstream projects get by with the assumption that res will always be a "bad request" containing the error message?

[bahamat]

I have two questions.

1. Is this a guarantee that it will never change?
2. Will something break if it's included? When logging (or whatever) I'd prefer to pass through what the actual error code was rather than making one up on my own.

[rjz]

Is this a guarantee that it will never change?

Insofar as it's covered in the spec (and semantically appropriate). In theory we'd bump version in response to a breaking change in the spec, as well, though there's (unfortunately) no hard guarantee on that.

Will something break if it's included?

I don't think so, as we would only be expanding the interface, but we would want to be sure we weren't inadvertently setting ourselves up for future trouble (e.g., by ensuring that only finalized res objects are being emitted, etc).

If we can get by with the current behavior (or a specific "webhook error" log entry as needed), great; if there are cases where we need direct access to res this is a small enough library that validation shouldn't be a huge issue either. Thoughts?

[bahamat]

I looked back through the code I was using for this, and to be quite honest, it was so long ago that I worked on this that I don't really remember exactly what I was going to use it for. I thought I knew, but after some review...it's at least not obvious. In any event, I'm unlikely to go back into my code at any time soon and make use of the res object even if it were there.

I'm ok with closing both the issue and the PR as unnecessary at this time.

[rjz]

Got it, and big thanks for checking back into this!