Skip to content

Commit a6d1460

Browse files
committed
Wrap onerror callbacks in try/catch to prevent masking transport errors
Since onerror is a user-provided callback, it could throw. Without defensive wrapping, a throwing onerror handler would mask the original transport error (the reject/throw would never execute). Addresses review feedback from @travisbreaks.
1 parent 9a8f2f8 commit a6d1460

4 files changed

Lines changed: 10 additions & 10 deletions

File tree

packages/client/src/client/stdio.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -258,7 +258,7 @@ export class StdioClientTransport implements Transport {
258258
}
259259
} catch (error) {
260260
const err = error instanceof Error ? error : new Error(String(error));
261-
this.onerror?.(err);
261+
try { this.onerror?.(err); } catch { /* handler error should not mask transport error */ }
262262
reject(err);
263263
}
264264
});

packages/client/src/client/websocket.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ export class WebSocketClientTransport implements Transport {
6565
try {
6666
if (!this._socket) {
6767
const err = new Error('Not connected');
68-
this.onerror?.(err);
68+
try { this.onerror?.(err); } catch { /* handler error should not mask transport error */ }
6969
reject(err);
7070
return;
7171
}
@@ -74,7 +74,7 @@ export class WebSocketClientTransport implements Transport {
7474
resolve();
7575
} catch (error) {
7676
const err = error instanceof Error ? error : new Error(String(error));
77-
this.onerror?.(err);
77+
try { this.onerror?.(err); } catch { /* handler error should not mask transport error */ }
7878
reject(err);
7979
}
8080
});

packages/server/src/server/stdio.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ export class StdioServerTransport implements Transport {
9797
}
9898
} catch (error) {
9999
const err = error instanceof Error ? error : new Error(String(error));
100-
this.onerror?.(err);
100+
try { this.onerror?.(err); } catch { /* handler error should not mask transport error */ }
101101
reject(err);
102102
}
103103
});

packages/server/src/server/streamableHttp.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,7 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
394394
primingEventId = await this._eventStore.storeEvent(streamId, {} as JSONRPCMessage);
395395
} catch (error) {
396396
const err = error as Error;
397-
this.onerror?.(err);
397+
try { this.onerror?.(err); } catch { /* handler error should not mask transport error */ }
398398
throw err;
399399
}
400400

@@ -962,7 +962,7 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
962962
// For standalone SSE streams, we can only send requests and notifications
963963
if (isJSONRPCResultResponse(message) || isJSONRPCErrorResponse(message)) {
964964
const err = new Error('Cannot send a response on a standalone SSE stream unless resuming a previous client request');
965-
this.onerror?.(err);
965+
try { this.onerror?.(err); } catch { /* handler error should not mask transport error */ }
966966
throw err;
967967
}
968968

@@ -975,7 +975,7 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
975975
eventId = await this._eventStore.storeEvent(this._standaloneSseStreamId, message);
976976
} catch (error) {
977977
const err = error as Error;
978-
this.onerror?.(err);
978+
try { this.onerror?.(err); } catch { /* handler error should not mask transport error */ }
979979
throw err;
980980
}
981981
}
@@ -997,7 +997,7 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
997997
const streamId = this._requestToStreamMapping.get(requestId);
998998
if (!streamId) {
999999
const err = new Error(`No connection established for request ID: ${String(requestId)}`);
1000-
this.onerror?.(err);
1000+
try { this.onerror?.(err); } catch { /* handler error should not mask transport error */ }
10011001
throw err;
10021002
}
10031003

@@ -1012,7 +1012,7 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
10121012
eventId = await this._eventStore.storeEvent(streamId, message);
10131013
} catch (error) {
10141014
const err = error as Error;
1015-
this.onerror?.(err);
1015+
try { this.onerror?.(err); } catch { /* handler error should not mask transport error */ }
10161016
throw err;
10171017
}
10181018
}
@@ -1030,7 +1030,7 @@ export class WebStandardStreamableHTTPServerTransport implements Transport {
10301030
if (allResponsesReady) {
10311031
if (!stream) {
10321032
const err = new Error(`No connection established for request ID: ${String(requestId)}`);
1033-
this.onerror?.(err);
1033+
try { this.onerror?.(err); } catch { /* handler error should not mask transport error */ }
10341034
throw err;
10351035
}
10361036
if (this._enableJsonResponse && stream.resolveJson) {

0 commit comments

Comments
 (0)