Draft implementation of server.on(handle-error, ...) #79
Conversation
|
|
src/mockttp.ts
Outdated
| * Subscribe to hear about requests that fail before successfully sending their | ||
| * initial parameters (the request line & headers). This will fire for requests | ||
| * that drop connections early, send invalid or too-long headers, or aren't | ||
| * correctly parseable in some form. |
src/mockttp.ts
Outdated
| @@ -576,7 +593,8 @@ export type SubscribableEvent = | |||
| | 'response' | |||
| | 'abort' | |||
| | 'tls-client-error' | |||
| | 'client-error'; | |||
| | 'client-error' | |||
| | 'handle-error'; | |||
There was a problem hiding this comment.
I think this should be handler-error - that's slightly clearer (since handle has other meanings, like file handles). This is basically an event you can subscribe to to hear about any request handlers which throw errors at any point during request processing.
| // TODO: This is just a copy of the previous client-error section. Needs to be finished. | ||
| 'handle-error': gql`subscription OnError { | ||
| failedRequest { | ||
| errorCode |
There was a problem hiding this comment.
To make this work, you'll need to:
- Expose it in the API in https://github.com/httptoolkit/mockttp/blob/main/src/admin/mockttp-admin-model.ts (you can just copy the other events there)
- Define it in the schema here - I'd suggest creating a new
Errortype withmessageandstackstring fields. - Adding a case to query for it in the client in here and some logic in here to turn the
{ message, stack }data into a properErrorobject.
I think that should be it, and then the identical API will work in browser & remote clients automatically too.
|
Oh, can you add tests for this please? I think you can base them on any of the other subscription types in here: https://github.com/httptoolkit/mockttp/tree/main/test/integration/subscriptions. You can probably use your ENOTFOUND dns error as a good test case to reproduce this I think 😄 |
|
Sure I'll try. But I have a question. I started to used the I'm not sure where should I add a description. I added it to |
|
What do you think about renaming |
|
It's not all server errors though - it's only errors during the request handling. There's all sorts of other server errors we can hit in other places, like low-level socket handling or during server startup. I've looked at adding an event for all this before, although I think I remember it being a bit more difficult than it sounds... Anyway, how about |
This means you need to export the ErrorLike type from the package root, so that people can use it themselves, like It's being a bit overly strict really, but it is still useful because it makes sure that all the types you might need are definitely directly accessible outside the package. To fix that you just need to add an |
src/mockttp.ts
Outdated
| * | ||
| * @category Events | ||
| */ | ||
| on(event: 'handle-error', callback: (error: ErrorLike) => void): Promise<void>; |
There was a problem hiding this comment.
Do you think it would be useful to include the request that failed here too as a second parameter, something like (error, req) => { ... }?
That would be as an InitiatedRequest I think, since we don't actually know if it completed. Or maybe we can do InitiatedRequest | CompletedRequest, I'm just not sure if we have an easy way to provide the completed data only if it's ready, without waiting if it's not...
…s RequestHandlerError type; added gql schema and builder; no tests yet
|
Updated PR, not finished yet: tests to come. |
It's a follow-up of this ticket: #78
Now it should be possible to hide errors for example:
Still needs works.