From 5dc8e455150af68b9af54a5c0360eb76f9674b4b Mon Sep 17 00:00:00 2001 From: Sri Krishna Paritala Date: Fri, 18 Aug 2023 14:42:04 +0700 Subject: [PATCH 1/3] Call `throw` and `return` on handler --- .../src/node-universal-handler.ts | 53 +++++++++++-------- 1 file changed, 32 insertions(+), 21 deletions(-) diff --git a/packages/connect-node/src/node-universal-handler.ts b/packages/connect-node/src/node-universal-handler.ts index 0397f8bd8..5e082c5b7 100644 --- a/packages/connect-node/src/node-universal-handler.ts +++ b/packages/connect-node/src/node-universal-handler.ts @@ -36,7 +36,7 @@ import { */ export type NodeHandlerFn = ( request: NodeServerRequest, - response: NodeServerResponse, + response: NodeServerResponse ) => void; /** @@ -55,12 +55,12 @@ export type NodeServerResponse = ( ) & { write( chunk: string | Uint8Array, - callback?: (err: Error | null | undefined) => void, + callback?: (err: Error | null | undefined) => void ): boolean; write( chunk: string | Uint8Array, encoding: BufferEncoding, - callback?: (err: Error | null | undefined) => void, + callback?: (err: Error | null | undefined) => void ): boolean; }; @@ -72,7 +72,7 @@ export type NodeServerResponse = ( */ export function universalRequestFromNodeRequest( nodeRequest: NodeServerRequest, - parsedJsonBody: JsonValue | undefined, + parsedJsonBody: JsonValue | undefined ): UniversalServerRequest { const encrypted = "encrypted" in nodeRequest.socket && nodeRequest.socket.encrypted; @@ -85,7 +85,7 @@ export function universalRequestFromNodeRequest( if (authority === undefined) { throw new ConnectError( "unable to determine request authority from Node.js server request", - Code.Internal, + Code.Internal ); } const body = @@ -136,21 +136,25 @@ export function universalRequestFromNodeRequest( */ export async function universalResponseToNodeResponse( universalResponse: UniversalServerResponse, - nodeResponse: NodeServerResponse, + nodeResponse: NodeServerResponse ): Promise { + const it = universalResponse.body?.[Symbol.asyncIterator](); + let isWriteError = false; try { - if (universalResponse.body !== undefined) { - for await (const chunk of universalResponse.body) { - // we deliberately send headers *in* this loop, not before, - // because we have to give the implementation a chance to - // set response headers - if (!nodeResponse.headersSent) { - nodeResponse.writeHead( - universalResponse.status, - webHeaderToNodeHeaders(universalResponse.header), - ); - } - await write(nodeResponse, chunk); + if (it !== undefined) { + let chunk = await it.next(); + isWriteError = true; + // we deliberately send headers after first read, not before, + // because we have to give the implementation a chance to + // set response headers + nodeResponse.writeHead( + universalResponse.status, + webHeaderToNodeHeaders(universalResponse.header) + ); + isWriteError = false; + for (; !chunk.done; chunk = await it.next()) { + isWriteError = true; + await write(nodeResponse, chunk.value); if ( "flush" in nodeResponse && typeof nodeResponse.flush == "function" @@ -166,17 +170,18 @@ export async function universalResponseToNodeResponse( // https://github.com/expressjs/compression/blob/ad5113b98cafe1382a0ece30bb4673707ac59ce7/index.js#L70 nodeResponse.flush(); } + isWriteError = false; } } if (!nodeResponse.headersSent) { nodeResponse.writeHead( universalResponse.status, - webHeaderToNodeHeaders(universalResponse.header), + webHeaderToNodeHeaders(universalResponse.header) ); } if (universalResponse.trailer) { nodeResponse.addTrailers( - webHeaderToNodeHeaders(universalResponse.trailer), + webHeaderToNodeHeaders(universalResponse.trailer) ); } await new Promise((resolve) => { @@ -186,12 +191,18 @@ export async function universalResponseToNodeResponse( nodeResponse.end(); }); } catch (e) { + // Report write errors to the handler. + if (isWriteError) { + it?.throw?.(e).catch(() => {}); + } throw connectErrorFromNodeReason(e); + } finally { + it?.return?.().catch(() => {}); } } async function* asyncIterableFromNodeServerRequest( - request: NodeServerRequest, + request: NodeServerRequest ): AsyncIterable { for await (const chunk of request) { yield chunk; From e9c61160144331066a949e5f22e2fc8588b2b0e8 Mon Sep 17 00:00:00 2001 From: Sri Krishna Paritala Date: Fri, 18 Aug 2023 14:43:32 +0700 Subject: [PATCH 2/3] fmt --- .../src/node-universal-handler.ts | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/connect-node/src/node-universal-handler.ts b/packages/connect-node/src/node-universal-handler.ts index 5e082c5b7..337d0de5e 100644 --- a/packages/connect-node/src/node-universal-handler.ts +++ b/packages/connect-node/src/node-universal-handler.ts @@ -36,7 +36,7 @@ import { */ export type NodeHandlerFn = ( request: NodeServerRequest, - response: NodeServerResponse + response: NodeServerResponse, ) => void; /** @@ -55,12 +55,12 @@ export type NodeServerResponse = ( ) & { write( chunk: string | Uint8Array, - callback?: (err: Error | null | undefined) => void + callback?: (err: Error | null | undefined) => void, ): boolean; write( chunk: string | Uint8Array, encoding: BufferEncoding, - callback?: (err: Error | null | undefined) => void + callback?: (err: Error | null | undefined) => void, ): boolean; }; @@ -72,7 +72,7 @@ export type NodeServerResponse = ( */ export function universalRequestFromNodeRequest( nodeRequest: NodeServerRequest, - parsedJsonBody: JsonValue | undefined + parsedJsonBody: JsonValue | undefined, ): UniversalServerRequest { const encrypted = "encrypted" in nodeRequest.socket && nodeRequest.socket.encrypted; @@ -85,7 +85,7 @@ export function universalRequestFromNodeRequest( if (authority === undefined) { throw new ConnectError( "unable to determine request authority from Node.js server request", - Code.Internal + Code.Internal, ); } const body = @@ -136,7 +136,7 @@ export function universalRequestFromNodeRequest( */ export async function universalResponseToNodeResponse( universalResponse: UniversalServerResponse, - nodeResponse: NodeServerResponse + nodeResponse: NodeServerResponse, ): Promise { const it = universalResponse.body?.[Symbol.asyncIterator](); let isWriteError = false; @@ -149,7 +149,7 @@ export async function universalResponseToNodeResponse( // set response headers nodeResponse.writeHead( universalResponse.status, - webHeaderToNodeHeaders(universalResponse.header) + webHeaderToNodeHeaders(universalResponse.header), ); isWriteError = false; for (; !chunk.done; chunk = await it.next()) { @@ -176,12 +176,12 @@ export async function universalResponseToNodeResponse( if (!nodeResponse.headersSent) { nodeResponse.writeHead( universalResponse.status, - webHeaderToNodeHeaders(universalResponse.header) + webHeaderToNodeHeaders(universalResponse.header), ); } if (universalResponse.trailer) { nodeResponse.addTrailers( - webHeaderToNodeHeaders(universalResponse.trailer) + webHeaderToNodeHeaders(universalResponse.trailer), ); } await new Promise((resolve) => { @@ -202,7 +202,7 @@ export async function universalResponseToNodeResponse( } async function* asyncIterableFromNodeServerRequest( - request: NodeServerRequest + request: NodeServerRequest, ): AsyncIterable { for await (const chunk of request) { yield chunk; From 890584deaba2bb8df13b651a4a8810aea93eb5b0 Mon Sep 17 00:00:00 2001 From: Sri Krishna Paritala Date: Fri, 18 Aug 2023 14:50:01 +0700 Subject: [PATCH 3/3] lint --- packages/connect-node/src/node-universal-handler.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/connect-node/src/node-universal-handler.ts b/packages/connect-node/src/node-universal-handler.ts index 337d0de5e..90173ee64 100644 --- a/packages/connect-node/src/node-universal-handler.ts +++ b/packages/connect-node/src/node-universal-handler.ts @@ -152,7 +152,7 @@ export async function universalResponseToNodeResponse( webHeaderToNodeHeaders(universalResponse.header), ); isWriteError = false; - for (; !chunk.done; chunk = await it.next()) { + for (; chunk.done !== true; chunk = await it.next()) { isWriteError = true; await write(nodeResponse, chunk.value); if (