Skip to content

Commit a71de2c

Browse files
committed
fix(websockets): deliver exception errors to native ws clients
BaseWsExceptionFilter used client.emit() to send exception payloads, which only works with socket.io. For native WebSocket clients (ws library), emit() is just EventEmitter.emit() and does not send data over the wire, causing exceptions to be silently swallowed. Add sendExceptionToClient() and isNativeWebSocket() methods to BaseWsExceptionFilter to detect the client type and use client.send() for native WebSocket clients. Update WsExceptionsHandler guard to allow clients with send() through to the exception filter. Closes #9056
1 parent 4500f04 commit a71de2c

File tree

4 files changed

+195
-10
lines changed

4 files changed

+195
-10
lines changed

integration/websockets/e2e/ws-gateway.spec.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { expect } from 'chai';
55
import * as WebSocket from 'ws';
66
import { ApplicationGateway } from '../src/app.gateway';
77
import { CoreGateway } from '../src/core.gateway';
8+
import { ErrorGateway } from '../src/error.gateway';
89
import { ExamplePathGateway } from '../src/example-path.gateway';
910
import { ServerGateway } from '../src/server.gateway';
1011
import { WsPathGateway } from '../src/ws-path.gateway';
@@ -273,6 +274,41 @@ describe('WebSocketGateway (WsAdapter)', () => {
273274
);
274275
});
275276

277+
it(`should handle WsException and send error to client`, async () => {
278+
app = await createNestApp(ErrorGateway);
279+
await app.listen(3000);
280+
281+
ws = new WebSocket('ws://localhost:8080');
282+
await new Promise(resolve => ws.on('open', resolve));
283+
284+
ws.send(
285+
JSON.stringify({
286+
event: 'push',
287+
data: {
288+
test: 'test',
289+
},
290+
}),
291+
);
292+
293+
await new Promise<void>((resolve, reject) => {
294+
const timeout = setTimeout(
295+
() => reject(new Error('Timeout: no error message received')),
296+
5000,
297+
);
298+
ws.on('message', data => {
299+
clearTimeout(timeout);
300+
const parsed = JSON.parse(data.toString());
301+
expect(parsed.event).to.be.eql('exception');
302+
expect(parsed.data).to.deep.include({
303+
status: 'error',
304+
message: 'test',
305+
});
306+
ws.close();
307+
resolve();
308+
});
309+
});
310+
});
311+
276312
afterEach(async function () {
277313
await app.close();
278314
});

packages/websockets/exceptions/base-ws-exception-filter.ts

Lines changed: 41 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ export class BaseWsExceptionFilter<
6464
});
6565
}
6666

67-
public handleError<TClient extends { emit: Function }>(
67+
public handleError<TClient extends { emit?: Function; send?: Function }>(
6868
client: TClient,
6969
exception: TError,
7070
cause: ErrorPayload['cause'],
@@ -77,7 +77,7 @@ export class BaseWsExceptionFilter<
7777
const result = exception.getError();
7878

7979
if (isObject(result)) {
80-
return client.emit('exception', result);
80+
return this.sendExceptionToClient(client, 'exception', result);
8181
}
8282

8383
const payload: ErrorPayload<unknown> = {
@@ -89,14 +89,12 @@ export class BaseWsExceptionFilter<
8989
payload.cause = this.options.causeFactory!(cause.pattern, cause.data);
9090
}
9191

92-
client.emit('exception', payload);
92+
this.sendExceptionToClient(client, 'exception', payload);
9393
}
9494

95-
public handleUnknownError<TClient extends { emit: Function }>(
96-
exception: TError,
97-
client: TClient,
98-
data: ErrorPayload['cause'],
99-
) {
95+
public handleUnknownError<
96+
TClient extends { emit?: Function; send?: Function },
97+
>(exception: TError, client: TClient, data: ErrorPayload['cause']) {
10098
const status = 'error';
10199
const payload: ErrorPayload<unknown> = {
102100
status,
@@ -107,14 +105,48 @@ export class BaseWsExceptionFilter<
107105
payload.cause = this.options.causeFactory!(data.pattern, data.data);
108106
}
109107

110-
client.emit('exception', payload);
108+
this.sendExceptionToClient(client, 'exception', payload);
111109

112110
if (!(exception instanceof IntrinsicException)) {
113111
const logger = BaseWsExceptionFilter.logger;
114112
logger.error(exception);
115113
}
116114
}
117115

116+
/**
117+
* Sends the exception payload to the client using the appropriate transport.
118+
* For native WebSocket clients (e.g., `ws` library), uses `client.send()`.
119+
* For socket.io clients, uses `client.emit()`.
120+
*
121+
* Override this method if you use a custom WebSocket adapter with a
122+
* different sending mechanism.
123+
*/
124+
protected sendExceptionToClient(
125+
client: any,
126+
event: string,
127+
payload: any,
128+
): void {
129+
if (this.isNativeWebSocket(client)) {
130+
if (client.readyState === 1) {
131+
client.send(JSON.stringify({ event, data: payload }));
132+
}
133+
} else if (typeof client.emit === 'function') {
134+
client.emit(event, payload);
135+
}
136+
}
137+
138+
/**
139+
* Determines whether the client is a native WebSocket instance (e.g., from
140+
* the `ws` library) rather than a socket.io socket.
141+
*/
142+
protected isNativeWebSocket(client: any): boolean {
143+
return (
144+
typeof client.send === 'function' &&
145+
typeof client.readyState === 'number' &&
146+
!client.nsp
147+
);
148+
}
149+
118150
public isExceptionObject(err: any): err is Error {
119151
return isObject(err) && !!(err as Error).message;
120152
}

packages/websockets/exceptions/ws-exceptions-handler.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@ export class WsExceptionsHandler extends BaseWsExceptionFilter {
1414

1515
public handle(exception: Error | WsException, host: ArgumentsHost) {
1616
const client = host.switchToWs().getClient();
17-
if (this.invokeCustomFilters(exception, host) || !client.emit) {
17+
if (
18+
this.invokeCustomFilters(exception, host) ||
19+
(!client.emit && !client.send)
20+
) {
1821
return;
1922
}
2023
super.catch(exception, host);

packages/websockets/test/exceptions/ws-exceptions-handler.spec.ts

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { ExecutionContextHost } from '@nestjs/core/helpers/execution-context-host';
22
import { expect } from 'chai';
33
import * as sinon from 'sinon';
4+
import { BaseWsExceptionFilter } from '../../exceptions/base-ws-exception-filter';
45
import { WsException } from '../../errors/ws-exception';
56
import { WsExceptionsHandler } from '../../exceptions/ws-exceptions-handler';
67

@@ -170,4 +171,117 @@ describe('WsExceptionsHandler', () => {
170171
});
171172
});
172173
});
174+
175+
describe('when client is a native WebSocket (ws library)', () => {
176+
let sendStub: sinon.SinonStub;
177+
let wsClient: {
178+
send: sinon.SinonStub;
179+
readyState: number;
180+
};
181+
let wsExecutionContextHost: ExecutionContextHost;
182+
183+
beforeEach(() => {
184+
sendStub = sinon.stub();
185+
wsClient = {
186+
send: sendStub,
187+
readyState: 1,
188+
};
189+
wsExecutionContextHost = new ExecutionContextHost([
190+
wsClient,
191+
data,
192+
pattern,
193+
]);
194+
});
195+
196+
it('should send JSON error via client.send when exception is unknown', () => {
197+
handler.handle(new Error(), wsExecutionContextHost);
198+
expect(sendStub.calledOnce).to.be.true;
199+
const sent = JSON.parse(sendStub.firstCall.args[0]);
200+
expect(sent).to.deep.equal({
201+
event: 'exception',
202+
data: {
203+
status: 'error',
204+
message: 'Internal server error',
205+
cause: {
206+
pattern,
207+
data,
208+
},
209+
},
210+
});
211+
});
212+
213+
it('should send JSON error via client.send when WsException has string message', () => {
214+
const message = 'Unauthorized';
215+
handler.handle(new WsException(message), wsExecutionContextHost);
216+
expect(sendStub.calledOnce).to.be.true;
217+
const sent = JSON.parse(sendStub.firstCall.args[0]);
218+
expect(sent).to.deep.equal({
219+
event: 'exception',
220+
data: {
221+
message,
222+
status: 'error',
223+
cause: {
224+
pattern,
225+
data,
226+
},
227+
},
228+
});
229+
});
230+
231+
it('should send JSON error via client.send when WsException has object message', () => {
232+
const message = { custom: 'Unauthorized' };
233+
handler.handle(new WsException(message), wsExecutionContextHost);
234+
expect(sendStub.calledOnce).to.be.true;
235+
const sent = JSON.parse(sendStub.firstCall.args[0]);
236+
expect(sent).to.deep.equal({
237+
event: 'exception',
238+
data: message,
239+
});
240+
});
241+
242+
it('should not send when readyState is not OPEN', () => {
243+
wsClient.readyState = 3;
244+
handler.handle(new WsException('test'), wsExecutionContextHost);
245+
expect(sendStub.notCalled).to.be.true;
246+
});
247+
});
248+
249+
describe('when client has neither emit nor send', () => {
250+
it('should bail out without throwing', () => {
251+
const bareClient = {};
252+
const bareCtx = new ExecutionContextHost([bareClient, data, pattern]);
253+
expect(() => handler.handle(new WsException('test'), bareCtx)).to.not
254+
.throw;
255+
});
256+
});
257+
});
258+
259+
describe('BaseWsExceptionFilter', () => {
260+
describe('isNativeWebSocket', () => {
261+
let filter: BaseWsExceptionFilter;
262+
263+
beforeEach(() => {
264+
filter = new BaseWsExceptionFilter();
265+
});
266+
267+
it('should return true for a raw ws client', () => {
268+
const wsClient = { send: () => {}, readyState: 1 };
269+
expect((filter as any).isNativeWebSocket(wsClient)).to.be.true;
270+
});
271+
272+
it('should return false for a socket.io client (has nsp)', () => {
273+
const ioClient = {
274+
send: () => {},
275+
readyState: 1,
276+
emit: () => {},
277+
nsp: {},
278+
};
279+
expect((filter as any).isNativeWebSocket(ioClient)).to.be.false;
280+
});
281+
282+
it('should return false for a client without send', () => {
283+
const client = { emit: () => {} };
284+
expect((filter as any).isNativeWebSocket(client)).to.be.false;
285+
});
286+
});
173287
});

0 commit comments

Comments
 (0)