Skip to content

Commit 8cf3745

Browse files
committed
pr feedback
Signed-off-by: Nik Nasr <nik@restate.dev>
1 parent 8fb04d5 commit 8cf3745

6 files changed

Lines changed: 17 additions & 111 deletions

File tree

packages/libs/restate-sdk/src/endpoint/handlers/fetch.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,8 @@ export function fetcher(handler: RestateHandler) {
4848
abortSignal: event.signal,
4949
})
5050
.catch((e) => {
51-
// wrapResponseWithSafety guarantees writeHead is called and
52-
// closes the output stream; anything reaching here is a
53-
// post-commit error.
51+
// Responses handle their own errors before rejecting; anything
52+
// reaching here is an unexpected failure — just log.
5453
const error = ensureError(e);
5554
const logger =
5655
tryCreateContextualLogger(

packages/libs/restate-sdk/src/endpoint/handlers/generic.ts

Lines changed: 4 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -72,16 +72,6 @@ const HOOK_CONTEXT_IS_PROCESSING_SYMBOL = Symbol.for(
7272
"@restatedev/restate-sdk/hooks.isProcessing"
7373
);
7474

75-
/**
76-
* Builds the Restate handler for an endpoint.
77-
*
78-
* Responses returned by `handle()` are wrapped via
79-
* {@link wrapResponseWithSafety}, which enforces exactly-once `writeHead`,
80-
* emits a 500 fallback if `process()` fails before `writeHead` or resolves
81-
* without committing a head, and closes the output stream on every path.
82-
* Adapters (node, fetch, lambda) can therefore treat transport failures as
83-
* logging concerns.
84-
*/
8575
export function createRestateHandler(
8676
endpoint: Endpoint,
8777
protocolMode: ProtocolMode,
@@ -94,78 +84,6 @@ export function createRestateHandler(
9484
);
9585
}
9686

97-
/**
98-
* Safety net around a `RestateResponse` returned by `_handle()`, applied
99-
* in `RestateHandlerImpl.handle()` before the response reaches any adapter.
100-
*
101-
* Guarantees to adapters:
102-
* - `writeHead` is called exactly once on the success path.
103-
* - If `process()` rejects before `writeHead`, a 500 `errorResponse` is
104-
* written instead (including input drain).
105-
* - If `process()` resolves without calling `writeHead`, same recovery.
106-
* - Post-writeHead errors propagate to the adapter for logging — the head
107-
* is already committed to the transport and can't be changed.
108-
*
109-
* Mirrors the existing pattern where `handle()` wraps user errors via
110-
* `errorResponse`; this extends that safety net to the `process()` phase.
111-
*/
112-
function wrapResponseWithSafety(inner: RestateResponse): RestateResponse {
113-
return {
114-
async process({ inputReader, outputWriter, writeHead, abortSignal }) {
115-
let committed = false;
116-
const safeWriteHead = (
117-
statusCode: number,
118-
headers: ResponseHeaders
119-
): void => {
120-
if (committed) {
121-
throw new Error("writeHead() called more than once");
122-
}
123-
committed = true;
124-
writeHead(statusCode, headers);
125-
};
126-
127-
try {
128-
try {
129-
await inner.process({
130-
inputReader,
131-
outputWriter,
132-
writeHead: safeWriteHead,
133-
abortSignal,
134-
});
135-
} catch (e) {
136-
if (committed) {
137-
// Head already on the wire — nothing structural the adapter can do.
138-
throw e;
139-
}
140-
await errorResponse(500, ensureError(e).message).process({
141-
inputReader,
142-
outputWriter,
143-
writeHead,
144-
abortSignal,
145-
});
146-
return;
147-
}
148-
149-
if (!committed) {
150-
await errorResponse(
151-
500,
152-
"Handler did not produce a response head"
153-
).process({
154-
inputReader,
155-
outputWriter,
156-
writeHead,
157-
abortSignal,
158-
});
159-
}
160-
} finally {
161-
// Ensure the output stream closes on every path. If it was already
162-
// closed (the usual case), close() rejects — which we swallow.
163-
await outputWriter.close().catch(() => {});
164-
}
165-
},
166-
};
167-
}
168-
16987
/**
17088
* This is the RestateHandler implementation
17189
*/
@@ -204,7 +122,7 @@ class RestateHandlerImpl implements RestateHandler {
204122
context?: AdditionalContext
205123
): RestateResponse {
206124
try {
207-
return wrapResponseWithSafety(this._handle(request, context));
125+
return this._handle(request, context);
208126
} catch (e) {
209127
const error = ensureError(e);
210128
(
@@ -216,11 +134,9 @@ class RestateHandlerImpl implements RestateHandler {
216134
).error(
217135
"Error while handling request: " + (error.stack ?? error.message)
218136
);
219-
return wrapResponseWithSafety(
220-
errorResponse(
221-
error instanceof RestateError ? error.code : 500,
222-
error.message
223-
)
137+
return errorResponse(
138+
error instanceof RestateError ? error.code : 500,
139+
error.message
224140
);
225141
}
226142
}

packages/libs/restate-sdk/src/endpoint/handlers/lambda.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -155,8 +155,8 @@ export class LambdaHandler {
155155
};
156156
}
157157

158-
// wrapResponseWithSafety guarantees writeHead was called, so this
159-
// resolves synchronously on the next microtask.
158+
// Responses always call writeHead, so this resolves on the next
159+
// microtask.
160160
const head = await headPromise;
161161

162162
const responseBodyBuffer = Buffer.concat(chunks);

packages/libs/restate-sdk/src/endpoint/handlers/types.ts

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,13 +36,8 @@ export interface RestateResponse {
3636
*
3737
* Implementations own the order of things: they must call {@link writeHead}
3838
* exactly once before the first {@link outputWriter.write}, read any
39-
* {@link inputReader} content they need, write the body, and finally call
40-
* {@link outputWriter.close}.
41-
*
42-
* {@link RestateHandler.handle} returns responses wrapped in a safety layer
43-
* that emits a 500 fallback if `process()` rejects before {@link writeHead}
44-
* or resolves without committing a head, and closes the output stream on
45-
* every path. Adapters can treat transport failures as logging concerns.
39+
* {@link inputReader} content they need, write the body, and call
40+
* {@link outputWriter.close} on every exit path (including error paths).
4641
*/
4742
process(value: {
4843
inputReader: InputReader;

packages/libs/restate-sdk/src/endpoint/handlers/utils.ts

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import type {
1212
import { createLogger, Logger } from "../../logging/logger.js";
1313
import { parseUrlComponents } from "../components.js";
1414
import { X_RESTATE_SERVER } from "../../user_agent.js";
15-
import { CompletablePromise } from "../../utils/completable_promise.js";
1615

1716
export function tryCreateContextualLogger(
1817
loggerTransport: LoggerTransport,
@@ -94,24 +93,21 @@ export function emptyInputReader(): InputReader {
9493

9594
/**
9695
* Bundles a `writeHead` callback with a Promise that resolves once the head
97-
* is committed. Used by adapters (fetch, lambda) that need to observe head
98-
* commit from outside the `process()` call.
99-
*
100-
* `writeHead` is expected to be called exactly once — enforced by the
101-
* safety layer in {@link RestateHandler.handle}, so there is no guard here.
96+
* is committed. Used by adapters (fetch, lambda) that need to observe the
97+
* head commit from outside the `process()` call.
10298
*/
10399
export function captureHead(): {
104100
writeHead: (statusCode: number, headers: ResponseHeaders) => void;
105101
head: Promise<{ statusCode: number; headers: ResponseHeaders }>;
106102
} {
107-
const ready = new CompletablePromise<{
103+
const { promise, resolve } = Promise.withResolvers<{
108104
statusCode: number;
109105
headers: ResponseHeaders;
110106
}>();
111107
return {
112108
writeHead: (statusCode, headers) => {
113-
ready.resolve({ statusCode, headers: { ...headers } });
109+
resolve({ statusCode, headers: { ...headers } });
114110
},
115-
head: ready.promise,
111+
head: promise,
116112
};
117113
}

packages/libs/restate-sdk/src/endpoint/node_endpoint.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -209,8 +209,8 @@ function nodeHandlerImpl(
209209
abortSignal: abortController.signal,
210210
})
211211
.catch((e) => {
212-
// wrapResponseWithSafety guarantees writeHead is called; anything
213-
// reaching here is a post-commit error. Nothing to do but log.
212+
// Responses handle their own errors before rejecting; anything
213+
// reaching here is an unexpected failure — just log.
214214
const error = ensureError(e);
215215
const logger =
216216
tryCreateContextualLogger(

0 commit comments

Comments
 (0)