Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft implementation of server.on(handle-error, ...) #79

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 39 additions & 1 deletion src/client/mockttp-admin-request-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,44 @@ export class MockttpAdminRequestBuilder {
body
}
}
}`,
// TODO: This is just a copy of the previous client-error section. Needs to be finished.
'handle-error': gql`subscription OnError {
failedRequest {
errorCode
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To make this work, you'll need to:

I think that should be it, and then the identical API will work in browser & remote clients automatically too.

request {
id
timingEvents
tags
protocol
httpVersion
method
url
path

${this.schema.typeHasField('ClientErrorRequest', 'rawHeaders')
? 'rawHeaders'
: 'headers'
}

${this.schema.asOptionalField('ClientErrorRequest', 'remoteIpAddress')},
${this.schema.asOptionalField('ClientErrorRequest', 'remotePort')},
}
response {
id
timingEvents
tags
statusCode
statusMessage

${this.schema.typeHasField('Response', 'rawHeaders')
? 'rawHeaders'
: 'headers'
}

body
}
}
}`
}[event];

Expand Down Expand Up @@ -367,4 +405,4 @@ export class MockttpAdminRequestBuilder {

return mockedEndpoint;
}
}
}
23 changes: 21 additions & 2 deletions src/mockttp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
} from "./types";
import type { RequestRuleData } from "./rules/requests/request-rule";
import type { WebSocketRuleData } from "./rules/websockets/websocket-rule";
import { ErrorLike } from "./util/error";

export type PortRange = { startPort: number, endPort: number };

Expand Down Expand Up @@ -415,6 +416,23 @@ export interface Mockttp {
*/
on(event: 'client-error', callback: (error: ClientError) => void): Promise<void>;

/**
* 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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment needs updating

*
* This is only useful in some niche use cases, such as logging all requests
* seen by the server, independently of the rules defined.
*
* The callback will be called asynchronously from request handling. This function
* returns a promise, and the callback is not guaranteed to be registered until
* the promise is resolved.
*
* @category Events
*/
on(event: 'handle-error', callback: (error: ErrorLike) => void): Promise<void>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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...


/**
* Adds the given rules to the server.
*
Expand Down Expand Up @@ -576,7 +594,8 @@ export type SubscribableEvent =
| 'response'
| 'abort'
| 'tls-client-error'
| 'client-error';
| 'client-error'
| 'handle-error';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


/**
* @hidden
Expand Down Expand Up @@ -677,4 +696,4 @@ export abstract class AbstractMockttp {
return new WebSocketRuleBuilder(this.addWebSocketRule);
}

}
}
56 changes: 31 additions & 25 deletions src/server/mockttp-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import { ServerMockedEndpoint } from "./mocked-endpoint";
import { createComboServer } from "./http-combo-server";
import { filter } from "../util/promise";
import { Mutable } from "../util/type-utils";
import { isErrorLike } from "../util/error";
import { ErrorLike, isErrorLike } from "../util/error";
import { makePropertyWritable } from "../util/util";

import {
Expand Down Expand Up @@ -277,6 +277,7 @@ export class MockttpServer extends AbstractMockttp implements Mockttp {
public on(event: 'tls-client-error', callback: (req: TlsRequest) => void): Promise<void>;
public on(event: 'tlsClientError', callback: (req: TlsRequest) => void): Promise<void>;
public on(event: 'client-error', callback: (error: ClientError) => void): Promise<void>;
public on(event: 'handle-error', callback: (error: ErrorLike) => void): Promise<void>;
public on(event: string, callback: (...args: any[]) => void): Promise<void> {
this.eventEmitter.on(event, callback);
return Promise.resolve();
Expand Down Expand Up @@ -464,32 +465,37 @@ export class MockttpServer extends AbstractMockttp implements Mockttp {
}
result = result || 'responded';
} catch (e) {
if (e instanceof AbortError) {
abort();

if (this.debug) {
console.error("Failed to handle request due to abort:", e);
}
} else {
console.error("Failed to handle request:",
this.debug
? e
: (isErrorLike(e) && e.message) || e
);
if (this.eventEmitter.listeners('handle-error').length > 0) {
this.eventEmitter.emit('handle-error', e);
}
else {
if (e instanceof AbortError) {
abort();

// Do whatever we can to tell the client we broke
try {
response.writeHead(
(isErrorLike(e) && e.statusCode) || 500,
(isErrorLike(e) && e.statusMessage) || 'Server error'
if (this.debug) {
console.error("Failed to handle request due to abort:", e);
}
} else {
console.error("Failed to handle request:",
this.debug
? e
: (isErrorLike(e) && e.message) || e
);
} catch (e) {}

try {
response.end((isErrorLike(e) && e.toString()) || e);
result = result || 'responded';
} catch (e) {
abort();
// Do whatever we can to tell the client we broke
try {
response.writeHead(
(isErrorLike(e) && e.statusCode) || 500,
(isErrorLike(e) && e.statusMessage) || 'Server error'
);
} catch (e) {}

try {
response.end((isErrorLike(e) && e.toString()) || e);
result = result || 'responded';
} catch (e) {
abort();
}
}
}
}
Expand Down Expand Up @@ -818,4 +824,4 @@ ${await this.suggestRule(request)}`
response: 'aborted' // These h2 errors get no app-level response, just a shutdown.
});
}
}
}
2 changes: 1 addition & 1 deletion src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -256,4 +256,4 @@ export interface ProxyEnvConfig {
// A slightly weird one: this is necessary because we export types that inherit from EventEmitter,
// so the docs include EventEmitter's methods, which @link to this type, that's otherwise not
// defined in this module. Reexporting the values avoids warnings for that.
export type defaultMaxListeners = typeof EventEmitter.defaultMaxListeners;
export type defaultMaxListeners = typeof EventEmitter.defaultMaxListeners;
3 changes: 2 additions & 1 deletion src/util/error.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// A server error
export type ErrorLike = Partial<Error> & {
// Various properties we might want to look for on errors:
code?: string;
Expand All @@ -15,4 +16,4 @@ export function isErrorLike(error: any): error is ErrorLike {
error.code ||
error.stack
)
}
}