Skip to content

Commit

Permalink
Fixes on client abort
Browse files Browse the repository at this point in the history
  • Loading branch information
ardatan committed Apr 26, 2024
1 parent 4466143 commit e6234df
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 4 deletions.
5 changes: 5 additions & 0 deletions .changeset/metal-bottles-flow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@whatwg-node/node-fetch": patch
---

Handle request errors properly
5 changes: 5 additions & 0 deletions .changeset/sweet-avocados-smoke.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@whatwg-node/server": patch
---

Do not call res.onAborted multiple times because it causes it to overwrite the previous listener, and use AbortSignal's abort event instead
1 change: 1 addition & 0 deletions packages/node-fetch/src/fetchNodeHttp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,7 @@ export function fetchNodeHttp<TResponseJSON = any, TRequestJSON = any>(
}
});
}
nodeResponse.once('error', reject);
const ponyfillResponse = new PonyfillResponse(responseBody, {
status: nodeResponse.statusCode,
statusText: nodeResponse.statusMessage,
Expand Down
6 changes: 5 additions & 1 deletion packages/server/src/createServerAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -255,9 +255,13 @@ function createServerAdapter<
resEnded = true;
return originalResEnd(data);
};
res.onAborted(() => {
const originalOnAborted = res.onAborted.bind(res);
originalOnAborted(function () {
signal.sendAbort();
});
res.onAborted = function (cb: () => void) {
signal.addEventListener('abort', cb);
};
const request = getRequestFromUWSRequest({
req,
res,
Expand Down
1 change: 1 addition & 0 deletions packages/server/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,7 @@ export function normalizeNodeRequest(
}
};

nodeResponse.once('error', closeEventListener);
nodeResponse.once('close', closeEventListener);

nodeResponse.once('finish', () => {
Expand Down
7 changes: 4 additions & 3 deletions packages/server/src/uwebsockets.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import type { Readable } from 'stream';
import type { FetchAPI } from './types.js';
import { ServerAdapterRequestAbortSignal } from './utils.js';

export interface UWSRequest {
getMethod(): string;
Expand Down Expand Up @@ -40,7 +41,7 @@ export function getRequestFromUWSRequest({ req, res, fetchAPI, signal }: GetRequ
if (method !== 'get' && method !== 'head') {
body = new fetchAPI.ReadableStream({});
const readable = (body as any).readable as Readable;
res.onAborted(() => {
signal.addEventListener('abort', () => {
readable.push(null);
});
res.onData(function (ab, isLast) {
Expand Down Expand Up @@ -71,7 +72,7 @@ export function getRequestFromUWSRequest({ req, res, fetchAPI, signal }: GetRequ
async function forwardResponseBodyToUWSResponse(
uwsResponse: UWSResponse,
fetchResponse: Response,
signal: AbortSignal,
signal: ServerAdapterRequestAbortSignal,
) {
for await (const chunk of fetchResponse.body as any as AsyncIterable<Uint8Array>) {
if (signal.aborted) {
Expand All @@ -89,7 +90,7 @@ async function forwardResponseBodyToUWSResponse(
export function sendResponseToUwsOpts(
uwsResponse: UWSResponse,
fetchResponse: Response,
signal: AbortSignal,
signal: ServerAdapterRequestAbortSignal,
) {
if (!fetchResponse) {
uwsResponse.writeStatus('404 Not Found');
Expand Down
35 changes: 35 additions & 0 deletions packages/server/test/node.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,41 @@ describe('Node Specific Cases', () => {
expect(abortListener).toHaveBeenCalledTimes(1);
});

it('handles AbortSignal correctly with streaming bodies', async () => {
const abortListener = jest.fn();
const adapterResponseDeferred = createDeferred<Response>();
function resolveAdapter() {
adapterResponseDeferred.resolve(
Response.json({
message: "You're so late!",
}),
);
}
const controller = new AbortController();
const serverAdapter = createServerAdapter(req => {
req.signal.addEventListener('abort', abortListener);
return req.text().then(() => {
controller.abort();
return adapterResponseDeferred.promise;
});
});
testServer.addOnceHandler(serverAdapter);
let error: Error | undefined;
try {
await fetch(testServer.url, {
method: 'POST',
signal: controller.signal,
body: 'Hello world!',
});
} catch (e: any) {
error = e;
}
expect(error).toBeDefined();
await sleep(300);
expect(abortListener).toHaveBeenCalledTimes(1);
resolveAdapter();
});

it('handles query parameters correctly', async () => {
const serverAdapter = createServerAdapter(req => {
const urlObj = new URL(req.url);
Expand Down

0 comments on commit e6234df

Please sign in to comment.