diff --git a/docs/README.md b/docs/README.md index 4b8a64e8..d8f77aa0 100644 --- a/docs/README.md +++ b/docs/README.md @@ -180,7 +180,7 @@ ___ Ƭ **EventError**: ``"error"`` -WebSocket connection had an error. +WebSocket connection had an error or client had an internal error. ___ @@ -192,10 +192,8 @@ ___ ▸ (`error`): `void` -The argument can be either an Error Event or an instance of Error, but to avoid -bundling DOM typings because the client can run in Node env too, you should assert -the type during implementation. Events dispatched from the WebSocket `onerror` can -be handler in this listener. +Events dispatched from the WebSocket `onerror` are handled in this listener, +as well as all internal client errors that could throw. **`category`** Client diff --git a/src/__tests__/client.ts b/src/__tests__/client.ts index 21d94897..4c49e762 100644 --- a/src/__tests__/client.ts +++ b/src/__tests__/client.ts @@ -206,18 +206,27 @@ it('should recieve optional connection ack payload in event handler', async (don }); }); -it('should close with error message during connecting issues', async () => { +it('should close with error message during connecting issues', async (done) => { + expect.assertions(4); + const { url } = await startTServer(); - const someerr = new Error('Welcome'); + const someErr = new Error('Welcome'); const client = createClient({ url, retryAttempts: 0, onNonLazyError: noop, on: { + closed: (event) => { + expect((event as CloseEvent).code).toBe(CloseCode.BadResponse); + expect((event as CloseEvent).reason).toBe('Welcome'); + expect((event as CloseEvent).wasClean).toBeTruthy(); + + done(); + }, connected: () => { // the `connected` listener is called right before successful connection - throw someerr; + throw someErr; }, }, }); @@ -227,10 +236,7 @@ it('should close with error message during connecting issues', async () => { }); await sub.waitForError((err) => { - const event = err as CloseEvent; - expect(event.code).toBe(CloseCode.BadResponse); - expect(event.reason).toBe('Welcome'); - expect(event.wasClean).toBeTruthy(); + expect(err).toBe(someErr); }); }); @@ -273,39 +279,53 @@ it('should pass the `connectionParams` through', async () => { }); }); -it('should close the socket if the `connectionParams` rejects or throws', async () => { +it('should close the socket if the `connectionParams` rejects or throws', async (done) => { + expect.assertions(8); + const server = await startTServer(); + const someErr = new Error('No auth?'); + let client = createClient({ url: server.url, retryAttempts: 0, onNonLazyError: noop, + on: { + closed: (event) => { + expect((event as CloseEvent).code).toBe(CloseCode.InternalClientError); + expect((event as CloseEvent).reason).toBe('No auth?'); + expect((event as CloseEvent).wasClean).toBeTruthy(); + }, + }, connectionParams: () => { - throw new Error('No auth?'); + throw someErr; }, }); let sub = tsubscribe(client, { query: '{ getValue }' }); await sub.waitForError((err) => { - const event = err as CloseEvent; - expect(event.code).toBe(CloseCode.InternalClientError); - expect(event.reason).toBe('No auth?'); - expect(event.wasClean).toBeTruthy(); + expect(err).toBe(someErr); }); client = createClient({ url: server.url, retryAttempts: 0, onNonLazyError: noop, - connectionParams: () => Promise.reject(new Error('No auth?')), + on: { + closed: (event) => { + expect((event as CloseEvent).code).toBe(CloseCode.InternalClientError); + expect((event as CloseEvent).reason).toBe('No auth?'); + expect((event as CloseEvent).wasClean).toBeTruthy(); + + done(); + }, + }, + connectionParams: () => Promise.reject(someErr), }); sub = tsubscribe(client, { query: '{ getValue }' }); await sub.waitForError((err) => { - const event = err as CloseEvent; - expect(event.code).toBe(CloseCode.InternalClientError); - expect(event.reason).toBe('No auth?'); - expect(event.wasClean).toBeTruthy(); + expect(err).toBe(someErr); }); }); @@ -508,7 +528,7 @@ it('should close socket with error on malformed request', async (done) => { }); it('should report close error even if complete message followed', async (done) => { - expect.assertions(4); + expect.assertions(3); const { url, server } = await startRawServer(); @@ -545,6 +565,8 @@ it('should report close error even if complete message followed', async (done) = closed: (err) => { expect((err as CloseEvent).code).toBe(CloseCode.BadResponse); expect((err as CloseEvent).reason).toBe('Invalid message'); + + done(); }, }, }); @@ -556,16 +578,123 @@ it('should report close error even if complete message followed', async (done) = { next: noop, error: (err) => { - expect((err as CloseEvent).code).toBe(CloseCode.BadResponse); - expect((err as CloseEvent).reason).toBe('Invalid message'); + expect((err as Error).message).toBe('Invalid message'); client.dispose(); - done(); }, complete: noop, }, ); }); +it('should report close causing internal client errors to listeners', async () => { + expect.assertions(4); + + const { url } = await startTServer(); + + const someError = new Error('Something went wrong!'); + + // internal client error + await new Promise((resolve) => + createClient({ + url, + // retryAttempts: 0, should fail immediately because error is thrown + lazy: false, + onNonLazyError: (err) => { + expect(err).toBe(someError); + resolve(); + }, + on: { + error: (err) => { + expect(err).toBe(someError); + }, + }, + connectionParams: () => { + throw someError; + }, + }), + ); + + // simulate bad response + await new Promise((resolve) => + createClient({ + url, + // retryAttempts: 0, should fail immediately because error is thrown + lazy: false, + onNonLazyError: (err) => { + expect(err).toBe(someError); + resolve(); + }, + on: { + error: (err) => { + expect(err).toBe(someError); + }, + message: () => { + throw someError; + }, + }, + }), + ); +}); + +it('should report close causing internal client errors to subscription sinks', async () => { + const { url } = await startTServer(); + + const someError = new Error('Something went wrong!'); + + // internal client error + await new Promise((resolve) => { + const client = createClient({ + url, + // retryAttempts: 0, should fail immediately because error is thrown + connectionParams: () => { + throw someError; + }, + }); + + client.subscribe( + { query: '{ getValue }' }, + { + next: noop, + complete: noop, + error: (err) => { + expect(err).toBe(someError); + resolve(); + }, + }, + ); + }); + + // simulate bad response + await new Promise((resolve) => { + let i = 0; + const client = createClient({ + url, + // retryAttempts: 0, should fail immediately because error is thrown + on: { + message: () => { + i++; + if (i === 2) { + // throw on second message to simulate bad responses while connected + throw someError; + } + }, + }, + }); + + client.subscribe( + { query: '{ getValue }' }, + { + next: noop, + complete: noop, + error: (err) => { + expect(err).toBe(someError); + resolve(); + }, + }, + ); + }); +}); + it('should limit the internal client error message size', async (done) => { const { url } = await startTServer(); @@ -1816,8 +1945,10 @@ describe('events', () => { lazy: false, retryAttempts: 0, onNonLazyError: (err) => { - // handle the websocket close event - expect((err as CloseEvent).code).toBe(1006); + // connection error + expect((err as ErrorEvent).message).toBe( + 'connect ECONNREFUSED 127.0.0.1:80', + ); done(); }, on: { @@ -1861,8 +1992,10 @@ describe('events', () => { next: noop, complete: noop, error: (err) => { - // handle the websocket close event - expect((err as CloseEvent).code).toBe(1006); + // connection error + expect((err as ErrorEvent).message).toBe( + 'connect ECONNREFUSED 127.0.0.1:80', + ); done(); }, }, diff --git a/src/client.ts b/src/client.ts index 43acd9ee..56ac15a8 100644 --- a/src/client.ts +++ b/src/client.ts @@ -78,7 +78,7 @@ export type EventMessage = 'message'; export type EventClosed = 'closed'; /** - * WebSocket connection had an error. + * WebSocket connection had an error or client had an internal error. * * @category Client */ @@ -166,10 +166,8 @@ export type EventMessageListener = (message: Message) => void; export type EventClosedListener = (event: unknown) => void; /** - * The argument can be either an Error Event or an instance of Error, but to avoid - * bundling DOM typings because the client can run in Node env too, you should assert - * the type during implementation. Events dispatched from the WebSocket `onerror` can - * be handler in this listener. + * Events dispatched from the WebSocket `onerror` are handled in this listener, + * as well as all internal client errors that could throw. * * @category Client */ @@ -535,7 +533,8 @@ export function createClient(options: ClientOptions): Client { }; }, emit(event: E, ...args: Parameters>) { - for (const listener of listeners[event]) { + // we copy the listeners so that unlistens dont "pull the rug under our feet" + for (const listener of [...listeners[event]]) { // @ts-expect-error: The args should fit listener(...args); } @@ -543,6 +542,23 @@ export function createClient(options: ClientOptions): Client { }; })(); + // invokes the callback either when an error or closed event is emitted, + // first one that gets called prevails, other emissions are ignored + function errorOrClosed(cb: (errOrEvent: unknown) => void) { + const listening = [ + // errors are fatal and more critical than close events, throw them first + emitter.on('error', (err) => { + listening.forEach((unlisten) => unlisten()); + cb(err); + }), + // closes can be graceful and not fatal, throw them second (if error didnt throw) + emitter.on('closed', (event) => { + listening.forEach((unlisten) => unlisten()); + cb(event); + }), + ]; + } + type Connected = [socket: WebSocket, throwOnClose: Promise]; let connecting: Promise | undefined, locks = 0, @@ -591,18 +607,14 @@ export function createClient(options: ClientOptions): Client { } } - socket.onerror = (err) => { - // we let the onclose reject the promise for correct retry handling - emitter.emit('error', err); - }; - - socket.onclose = (event) => { + errorOrClosed((errOrEvent) => { connecting = undefined; clearTimeout(connectionAckTimeout); clearTimeout(queuedPing); - emitter.emit('closed', event); - denied(event); - }; + denied(errOrEvent); + }); + socket.onerror = (err) => emitter.emit('error', err); + socket.onclose = (event) => emitter.emit('closed', event); socket.onopen = async () => { try { @@ -640,6 +652,7 @@ export function createClient(options: ClientOptions): Client { enqueuePing(); // enqueue ping (noop if disabled) } catch (err) { + emitter.emit('error', err); socket.close( CloseCode.InternalClientError, limitCloseReason( @@ -691,12 +704,11 @@ export function createClient(options: ClientOptions): Client { retries = 0; // reset the retries on connect connected([ socket, - new Promise((_, closed) => - socket.addEventListener('close', closed), - ), + new Promise((_, reject) => errorOrClosed(reject)), ]); } catch (err) { socket.onmessage = null; // stop reading messages as soon as reading breaks once + emitter.emit('error', err); socket.close( CloseCode.BadResponse, limitCloseReason(