From cc0fc9e54d2db8148fe22c35a39721acaa3ca5e9 Mon Sep 17 00:00:00 2001 From: Marcos Passos Date: Fri, 2 Aug 2024 11:45:16 -0600 Subject: [PATCH 01/20] Refactor the SDK to use the new HTTP event tracker --- package-lock.json | 23 -- package.json | 2 - src/channel/beaconSocketChannel.ts | 153 ---------- src/channel/httpBeaconChannel.ts | 115 ++++++++ src/channel/index.ts | 3 +- src/channel/socketChannel.ts | 217 -------------- src/container.ts | 29 +- src/sdk.ts | 3 +- test/channel/beaconSocketChannel.test.ts | 347 ----------------------- test/channel/httpBeaconChannel.test.ts | 289 +++++++++++++++++++ test/channel/socketChannel.test.ts | 233 --------------- test/container.test.ts | 33 ++- test/sdk.test.ts | 166 +++++------ 13 files changed, 513 insertions(+), 1100 deletions(-) delete mode 100644 src/channel/beaconSocketChannel.ts create mode 100644 src/channel/httpBeaconChannel.ts delete mode 100644 src/channel/socketChannel.ts delete mode 100644 test/channel/beaconSocketChannel.test.ts create mode 100644 test/channel/httpBeaconChannel.test.ts delete mode 100644 test/channel/socketChannel.test.ts diff --git a/package-lock.json b/package-lock.json index 25ae41f1..93534944 100644 --- a/package-lock.json +++ b/package-lock.json @@ -20,8 +20,6 @@ "jest": "^29.3.1", "jest-environment-jsdom": "^29.3.1", "jest-extended": "^4.0.0", - "jest-websocket-mock": "^2.4.0", - "mock-socket": "^9.1.5", "node-fetch": "^2.6.7", "ts-jest": "^29.0.3", "typescript": "^5.0.0" @@ -6152,17 +6150,6 @@ "node": "^14.15.0 || ^16.10.0 || >=18.0.0" } }, - "node_modules/jest-websocket-mock": { - "version": "2.5.0", - "resolved": "https://registry.npmjs.org/jest-websocket-mock/-/jest-websocket-mock-2.5.0.tgz", - "integrity": "sha512-a+UJGfowNIWvtIKIQBHoEWIUqRxxQHFx4CXT+R5KxxKBtEQ5rS3pPOV/5299sHzqbmeCzxxY5qE4+yfXePePig==", - "dev": true, - "license": "MIT", - "dependencies": { - "jest-diff": "^29.2.0", - "mock-socket": "^9.3.0" - } - }, "node_modules/jest-worker": { "version": "29.7.0", "resolved": "https://registry.npmjs.org/jest-worker/-/jest-worker-29.7.0.tgz", @@ -6619,16 +6606,6 @@ "url": "https://github.com/sponsors/ljharb" } }, - "node_modules/mock-socket": { - "version": "9.3.1", - "resolved": "https://registry.npmjs.org/mock-socket/-/mock-socket-9.3.1.tgz", - "integrity": "sha512-qxBgB7Qa2sEQgHFjj0dSigq7fX4k6Saisd5Nelwp2q8mlbAFh5dHV9JTTlF8viYJLSSWgMCZFUom8PJcMNBoJw==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">= 8" - } - }, "node_modules/ms": { "version": "2.1.2", "resolved": "https://registry.npmjs.org/ms/-/ms-2.1.2.tgz", diff --git a/package.json b/package.json index 512bd29d..a64a98d8 100644 --- a/package.json +++ b/package.json @@ -39,8 +39,6 @@ "jest": "^29.3.1", "jest-environment-jsdom": "^29.3.1", "jest-extended": "^4.0.0", - "jest-websocket-mock": "^2.4.0", - "mock-socket": "^9.1.5", "node-fetch": "^2.6.7", "ts-jest": "^29.0.3", "typescript": "^5.0.0" diff --git a/src/channel/beaconSocketChannel.ts b/src/channel/beaconSocketChannel.ts deleted file mode 100644 index b96f7b8f..00000000 --- a/src/channel/beaconSocketChannel.ts +++ /dev/null @@ -1,153 +0,0 @@ -import {ChannelListener, DuplexChannel} from './channel'; -import {Envelope} from './guaranteedChannel'; -import {Logger, LoggerFactory, NullLogger} from '../logging'; -import {CidAssigner} from '../cid'; - -export interface DuplexChannelFactory { - (url: string, logger: Logger): DuplexChannel; -} - -type Configuration = { - logger?: Logger, - loggerFactory?: LoggerFactory, - tokenParameter: string, - trackerEndpointUrl: string, - channelFactory: DuplexChannelFactory, - cidAssigner: CidAssigner, - cidParameter: string, -}; - -type Violation = { - message: string, - path: string, -}; - -type Confirmation = { - receiptId: string | null, - violations?: Violation[], -}; - -export class BeaconSocketChannel implements DuplexChannel> { - private readonly socketFactory: DuplexChannelFactory; - - private readonly logger: Logger; - - private readonly loggerFactory: LoggerFactory; - - private readonly cidAssigner: CidAssigner; - - private readonly cidParameter: string; - - private readonly tokenParameter: string; - - private readonly trackerEndpointUrl: string; - - private readonly listeners: Array> = []; - - private socketChannel?: DuplexChannel; - - private token?: string; - - private connectionIndex = 0; - - public constructor(configuration: Configuration) { - this.socketFactory = configuration.channelFactory; - this.logger = configuration.logger ?? new NullLogger(); - this.loggerFactory = configuration.loggerFactory ?? ((): Logger => new NullLogger()); - this.cidAssigner = configuration.cidAssigner; - this.cidParameter = configuration.cidParameter; - this.trackerEndpointUrl = configuration.trackerEndpointUrl; - this.tokenParameter = configuration.tokenParameter; - this.notify = this.notify.bind(this); - } - - public async publish({id: receiptId, message}: Envelope): Promise { - const {token, timestamp, context, payload} = JSON.parse(message); - - if (this.token !== token || this.socketChannel === undefined) { - if (this.socketChannel !== undefined) { - this.logger.info('Connection no longer valid for current message.'); - - this.socketChannel.unsubscribe(this.notify); - - await this.socketChannel.close(); - } - - this.token = token; - this.socketChannel = await this.createSocketChannel(token); - } - - return this.socketChannel.publish( - JSON.stringify({ - receiptId: receiptId, - originalTime: timestamp, - departureTime: Date.now(), - context: context, - payload: payload, - }), - ); - } - - private async createSocketChannel(token?: string): Promise> { - const endpoint = new URL(this.trackerEndpointUrl); - - endpoint.searchParams.append(this.cidParameter, await this.cidAssigner.assignCid()); - - if (token !== undefined) { - endpoint.searchParams.append(this.tokenParameter, token); - } - - const channel: DuplexChannel = this.socketFactory( - endpoint.toString(), - this.loggerFactory(`WebSocket#${this.connectionIndex}`), - ); - - this.connectionIndex += 1; - - channel.subscribe(this.notify); - - return channel; - } - - public subscribe(listener: ChannelListener): void { - if (!this.listeners.includes(listener)) { - this.listeners.push(listener); - } - } - - public unsubscribe(listener: ChannelListener): void { - const index = this.listeners.indexOf(listener); - - if (index >= 0) { - this.listeners.splice(index, 1); - } - } - - private notify(message: string): void { - let confirmation: Confirmation; - - try { - confirmation = JSON.parse(message); - } catch { - this.logger.error('Invalid JSON message received.'); - - return; - } - - const {violations = [], receiptId} = confirmation; - - violations.forEach(violation => this.logger.error(violation.message)); - - if (receiptId !== null) { - this.listeners.forEach(dispatch => dispatch(receiptId)); - } - } - - public close(): Promise { - if (this.socketChannel === undefined) { - return Promise.resolve(); - } - - return this.socketChannel.close(); - } -} diff --git a/src/channel/httpBeaconChannel.ts b/src/channel/httpBeaconChannel.ts new file mode 100644 index 00000000..4ec18ca1 --- /dev/null +++ b/src/channel/httpBeaconChannel.ts @@ -0,0 +1,115 @@ +import {ChannelListener, DuplexChannel} from './channel'; +import {Envelope} from './guaranteedChannel'; +import {Logger, NullLogger} from '../logging'; +import {CidAssigner} from '../cid'; +import {formatMessage} from '../error'; + +export type Configuration = { + appId: string, + endpointUrl: string, + cidAssigner: CidAssigner, + logger?: Logger, +}; + +type ApiProblem = { + type: string, + title: string, + status: number, + detail: string, +}; + +export class HttpBeaconChannel implements DuplexChannel> { + private static readonly NON_RETRYABLE_STATUSES: ReadonlySet = new Set([ + 403, // API usage limit exceeded + 401, // Invalid token + ]); + + private readonly configuration: Omit; + + private readonly logger: Logger; + + private readonly listeners: Array> = []; + + private closed = false; + + public constructor({logger = new NullLogger(), ...configuration}: Configuration) { + this.configuration = configuration; + this.logger = logger; + } + + public async publish({id: receiptId, message}: Envelope): Promise { + if (this.closed) { + return Promise.reject(new Error('Channel is closed')); + } + + const {token, timestamp, context, payload} = JSON.parse(message); + const {endpointUrl, appId, cidAssigner} = this.configuration; + + return fetch(endpointUrl, { + method: 'POST', + headers: { + 'X-Application-Id': appId, + 'X-Client-Id': await cidAssigner.assignCid(), + ...(token !== undefined ? {'X-Token': token} : {}), + }, + body: JSON.stringify({ + context: context, + payload: payload, + originalTime: timestamp, + departureTime: Date.now(), + }), + }).then(async response => { + if (response.ok) { + this.notify(receiptId); + + return; + } + + const problem: ApiProblem = await response.json().catch( + () => ({ + type: 'https://croct.help/api/event-tracker#unexpected-error', + title: response.statusText, + status: response.status, + }), + ); + + if (HttpBeaconChannel.NON_RETRYABLE_STATUSES.has(problem.status)) { + this.logger.error(`Beacon rejected with non-retryable status: ${problem.title}`); + + this.notify(receiptId); + + return Promise.resolve(); + } + + return Promise.reject(new Error(problem.title)); + }).catch(error => { + this.logger.error(`Failed to publish beacon: ${formatMessage(error)}`); + + return Promise.reject(error); + }); + } + + public subscribe(listener: ChannelListener): void { + if (!this.listeners.includes(listener)) { + this.listeners.push(listener); + } + } + + public unsubscribe(listener: ChannelListener): void { + const index = this.listeners.indexOf(listener); + + if (index >= 0) { + this.listeners.splice(index, 1); + } + } + + private notify(receiptId: string): void { + this.listeners.forEach(dispatch => dispatch(receiptId)); + } + + public close(): Promise { + this.closed = true; + + return Promise.resolve(); + } +} diff --git a/src/channel/index.ts b/src/channel/index.ts index 514e6964..518b2dd4 100644 --- a/src/channel/index.ts +++ b/src/channel/index.ts @@ -1,8 +1,7 @@ export * from './channel'; -export {BeaconSocketChannel, DuplexChannelFactory} from './beaconSocketChannel'; export {EncodedChannel} from './encodedChannel'; export {GuaranteedChannel} from './guaranteedChannel'; export {QueuedChannel} from './queuedChannel'; export {RetryChannel} from './retryChannel'; export {SandboxChannel} from './sandboxChannel'; -export {SocketChannel} from './socketChannel'; +export {HttpBeaconChannel} from './httpBeaconChannel'; diff --git a/src/channel/socketChannel.ts b/src/channel/socketChannel.ts deleted file mode 100644 index aa6b1109..00000000 --- a/src/channel/socketChannel.ts +++ /dev/null @@ -1,217 +0,0 @@ -import {Logger, NullLogger} from '../logging'; -import {ChannelListener, DuplexChannel} from './channel'; -import {formatCause} from '../error'; - -type Input = string | ArrayBufferLike | Blob | ArrayBufferView; -type Output = string | ArrayBuffer | Blob; - -type Options = { - closeTimeout: number, - connectionTimeout: number, - protocols: string | string[], - binaryType?: BinaryType, -}; - -export type Configuration = Partial & { - url: string, - logger?: Logger, -}; - -export class SocketChannel implements DuplexChannel { - private readonly url: string; - - private readonly logger: Logger; - - private readonly options: Options; - - private readonly listeners: Array> = []; - - private connection?: Promise; - - private closed = false; - - public constructor({url, logger, ...options}: Configuration) { - this.url = url; - this.logger = logger ?? new NullLogger(); - this.options = { - ...options, - closeTimeout: options.closeTimeout ?? 5000, - connectionTimeout: options.connectionTimeout ?? 10000, - protocols: options.protocols ?? [], - }; - } - - public get connected(): Promise { - if (this.connection === undefined) { - return Promise.resolve(false); - } - - return this.connection.then(() => true, () => false); - } - - public publish(message: O): Promise { - return this.connect().then(socket => { - socket.send(message); - - this.logger.debug('Message sent.'); - }); - } - - public subscribe(listener: ChannelListener): void { - if (!this.listeners.includes(listener)) { - this.listeners.push(listener); - } - } - - public unsubscribe(listener: ChannelListener): void { - const index = this.listeners.indexOf(listener); - - if (index >= 0) { - this.listeners.splice(index, 1); - } - } - - private notify(message: I): void { - this.listeners.forEach(dispatch => dispatch(message)); - } - - private connect(): Promise { - if (this.closed) { - return Promise.reject(new Error('Channel has been closed.')); - } - - if (this.connection !== undefined) { - return this.connection - .then(connection => { - const state = connection.readyState; - - if (state === WebSocket.OPEN) { - return connection; - } - - throw new Error('Connection lost.'); - }) - .catch(() => { - // Reconnect - delete this.connection; - - return this.connect(); - }); - } - - this.connection = new Promise((resolve, reject): void => { - this.logger.debug('Connecting...'); - - const connection = new window.WebSocket(this.url, this.options.protocols); - - if (this.options.binaryType !== undefined) { - connection.binaryType = this.options.binaryType; - } - - const abortListener = (): void => { - const reason = 'Maximum connection timeout reached.'; - - this.logger.error(reason); - - reject(new Error(reason)); - - connection.close(1000, reason); - }; - - const abortTimer: number = window.setTimeout(abortListener, this.options.connectionTimeout); - - const openListener = (): void => { - window.clearTimeout(abortTimer); - - this.logger.info('Connection established.'); - - connection.removeEventListener('open', openListener); - - resolve(connection); - }; - - const errorListener = (): void => { - if (!this.closed) { - this.logger.error('Connection error.'); - } - }; - - const messageListener = (event: MessageEvent): void => { - this.logger.debug('Message received.'); - - this.notify(event.data); - }; - - const closeListener = (event: CloseEvent): void => { - window.clearTimeout(abortTimer); - - const reason = `${formatCause(event.reason ?? 'unknown')} (code ${event.code})`; - const message = `Connection has been closed, reason: ${reason}`; - - if (!this.closed) { - this.logger.info(message); - } - - connection.removeEventListener('open', openListener); - connection.removeEventListener('error', errorListener); - connection.removeEventListener('close', closeListener); - connection.removeEventListener('message', messageListener); - - reject(new Error(message)); - }; - - connection.addEventListener('open', openListener, {once: true}); - connection.addEventListener('close', closeListener, {once: true}); - connection.addEventListener('error', errorListener); - connection.addEventListener('message', messageListener); - }); - - return this.connection; - } - - public close(): Promise { - this.logger.debug('Closing connection...'); - - return new Promise((resolve, reject): void => { - this.closed = true; - - if (this.connection === undefined) { - this.logger.debug('Connection is not open.'); - - resolve(); - - return; - } - - this.connection.then( - (connection): void => { - let abortTimer: number | undefined; - - const abort = (): void => { - this.logger.warn('Connection could not be closed within the timeout period.'); - - reject(new Error('Maximum close timeout reached.')); - }; - - const close = (): void => { - window.clearTimeout(abortTimer); - - this.logger.info('Connection gracefully closed.'); - - resolve(); - }; - - connection.addEventListener('close', close, {once: true}); - connection.close(1000, 'Deliberate disconnection.'); - - abortTimer = window.setTimeout(abort, this.options.closeTimeout); - }, - () => { - this.logger.info('Connection closed.'); - - resolve(); - }, - ); - }); - } -} diff --git a/src/container.ts b/src/container.ts index c5b3e647..836243df 100644 --- a/src/container.ts +++ b/src/container.ts @@ -20,13 +20,12 @@ import { RetryChannel, GuaranteedChannel, EncodedChannel, - BeaconSocketChannel, - SocketChannel, SandboxChannel, } from './channel'; import {ContentFetcher} from './contentFetcher'; import {CookieCache, CookieCacheConfiguration} from './cache/cookieCache'; import {FilteredLogger} from './logging/filteredLogger'; +import {HttpBeaconChannel} from './channel/httpBeaconChannel'; export type DependencyResolver = (container: Container) => T; @@ -226,23 +225,18 @@ export class Container { const queuedChannel = new QueuedChannel( new RetryChannel({ channel: new GuaranteedChannel({ - channel: new BeaconSocketChannel({ - trackerEndpointUrl: `${trackerEndpointUrl}/${appId}`, - tokenParameter: 'token', - loggerFactory: this.getLogger.bind(this), - logger: channelLogger, - channelFactory: (url, logger): SocketChannel => ( - new SocketChannel({url: url, logger: logger}) - ), + channel: new HttpBeaconChannel({ + appId: appId, + endpointUrl: trackerEndpointUrl, cidAssigner: this.getCidAssigner(), - cidParameter: 'clientId', + logger: channelLogger, }), stamper: new TimeStamper(), ackTimeout: 10000, logger: channelLogger, }), retryPolicy: new BackoffPolicy({ - minRetryDelay: 1000, // 1 second + minRetryDelay: 3000, // 1 second maxRetryDelay: 60 * 1000, // 60 seconds backoffFactor: 1.5, // 1.5 ^ attempt backoffJitter: 1, // add randomness @@ -300,12 +294,9 @@ export class Container { } private createBeaconQueue(): MonitoredQueue { - const context = this.getContext(); - const tab = context.getTab(); - return new MonitoredQueue( new CapacityRestrictedQueue( - new PersistentQueue(this.getGlobalTabStorage('queue'), tab.id), + new PersistentQueue(this.getGlobalTabStorage('queue')), this.configuration.beaconQueueSize, ), this.getLogger('BeaconQueue'), @@ -349,9 +340,9 @@ export class Container { } private resolveStorageNamespace(namespace: string, ...subnamespace: string[]): string { - return `croct[${this.configuration - .appId - .toLowerCase()}].${[namespace].concat(subnamespace).join('.')}`; + const {appId} = this.configuration; + + return `croct[${appId.toLowerCase()}].${[namespace].concat(subnamespace).join('.')}`; } private getLocalStorage(): Storage { diff --git a/src/sdk.ts b/src/sdk.ts index 0209ebf2..f6c5c11c 100644 --- a/src/sdk.ts +++ b/src/sdk.ts @@ -75,13 +75,12 @@ export class Sdk { } const baseHttpEndpoint = baseEndpointUrl.replace(/\/+$/, ''); - const baseWsEndpoint = baseHttpEndpoint.replace(/^http/i, 'ws'); const container = new Container({ ...containerConfiguration, evaluationBaseEndpointUrl: baseHttpEndpoint, contentBaseEndpointUrl: baseHttpEndpoint, - trackerEndpointUrl: `${baseWsEndpoint}/client/web/connect`, + trackerEndpointUrl: `${baseHttpEndpoint}/client/web/track`, cidAssignerEndpointUrl: cidAssignerEndpointUrl ?? `${baseHttpEndpoint}/client/web/cid`, beaconQueueSize: containerConfiguration.beaconQueueSize ?? 100, eventMetadata: eventMetadata, diff --git a/test/channel/beaconSocketChannel.test.ts b/test/channel/beaconSocketChannel.test.ts deleted file mode 100644 index 254ed8f0..00000000 --- a/test/channel/beaconSocketChannel.test.ts +++ /dev/null @@ -1,347 +0,0 @@ -import {BeaconSocketChannel, DuplexChannelFactory, SandboxChannel, DuplexChannel} from '../../src/channel'; -import {Envelope} from '../../src/channel/guaranteedChannel'; -import {Beacon, BeaconPayload, TrackingEventContext} from '../../src/trackingEvents'; -import {FixedAssigner} from '../../src/cid'; - -describe('A beacon socket channel', () => { - afterEach(() => { - jest.restoreAllMocks(); - }); - - const context: TrackingEventContext = { - tabId: '123', - url: 'https://localhost', - metadata: { - foo: 'bar', - }, - }; - - const payload: BeaconPayload = { - type: 'nothingChanged', - sinceTime: 0, - }; - - it('should publish messages on the output channel', async () => { - const date = jest.spyOn(Date, 'now'); - const now = Date.now(); - - date.mockReturnValue(now); - - const socketChannel = new SandboxChannel(); - const channelFactory: DuplexChannelFactory = jest.fn().mockReturnValue(socketChannel); - - const channel = new BeaconSocketChannel({ - channelFactory: channelFactory, - tokenParameter: 'token', - trackerEndpointUrl: 'ws://localhost:8080', - cidParameter: 'clientId', - cidAssigner: new FixedAssigner('123'), - }); - - const beacon: Beacon = { - timestamp: 123456789, - context: context, - payload: payload, - }; - - const message: Envelope = { - id: '123', - message: JSON.stringify(beacon), - }; - - await channel.publish(message); - - expect(channelFactory).toHaveBeenCalledWith('ws://localhost:8080/?clientId=123', {}); - expect(socketChannel.messages).toHaveLength(1); - - const [publishedMessage] = socketChannel.messages; - const expectedMessage = { - receiptId: '123', - originalTime: 123456789, - departureTime: now, - context: context, - payload: payload, - }; - - expect(JSON.parse(publishedMessage)).toStrictEqual(expectedMessage); - }); - - it('should establish a new connection when the token changes', async () => { - const date = jest.spyOn(Date, 'now'); - const now = Date.now(); - - date.mockReturnValue(now); - - const firstSocketChannel = new SandboxChannel(); - const secondSocketChannel = new SandboxChannel(); - - const channelFactory: DuplexChannelFactory = jest.fn() - .mockReturnValueOnce(firstSocketChannel) - .mockReturnValueOnce(secondSocketChannel); - - const channel = new BeaconSocketChannel({ - channelFactory: channelFactory, - tokenParameter: 'token', - trackerEndpointUrl: 'ws://localhost:8080', - cidParameter: 'clientId', - cidAssigner: new FixedAssigner('123'), - }); - - const firstBeacon: Beacon = { - timestamp: 123456789, - context: context, - payload: payload, - }; - - const firstMessage: Envelope = { - id: '123', - message: JSON.stringify(firstBeacon), - }; - - await channel.publish(firstMessage); - - expect(channelFactory).toHaveBeenLastCalledWith('ws://localhost:8080/?clientId=123', {}); - - expect(firstSocketChannel.messages).toHaveLength(1); - - const [firstPublishedMessage] = firstSocketChannel.messages; - const firstExpectedMessage = { - receiptId: '123', - originalTime: 123456789, - departureTime: now, - payload: payload, - context: context, - }; - - expect(JSON.parse(firstPublishedMessage)).toStrictEqual(firstExpectedMessage); - - const secondBeacon: Beacon = { - token: 'some-token', - timestamp: 234567890, - context: context, - payload: payload, - }; - - const secondMessage: Envelope = { - id: '456', - message: JSON.stringify(secondBeacon), - }; - - await channel.publish(secondMessage); - - expect(channelFactory).toHaveBeenCalledWith('ws://localhost:8080/?clientId=123&token=some-token', {}); - - expect(secondSocketChannel.messages).toHaveLength(1); - - const [secondPublishedMessage] = secondSocketChannel.messages; - const secondExpectedMessage = { - receiptId: '456', - departureTime: now, - originalTime: 234567890, - context: context, - payload: payload, - }; - - expect(JSON.parse(secondPublishedMessage)).toStrictEqual(secondExpectedMessage); - }); - - it('should fail if an error occurs while closing the current connection', async () => { - const date = jest.spyOn(Date, 'now'); - const now = Date.now(); - - date.mockReturnValue(now); - - const error = new Error('Error while closing.'); - const publish = jest.fn(); - const duplexChannel: DuplexChannel = { - close: jest.fn().mockRejectedValue(error), - publish: publish, - subscribe: jest.fn(), - unsubscribe: jest.fn(), - }; - - const channel = new BeaconSocketChannel({ - channelFactory: (): DuplexChannel => duplexChannel, - tokenParameter: 'token', - trackerEndpointUrl: 'ws://localhost:8080', - cidParameter: 'clientId', - cidAssigner: new FixedAssigner('123'), - }); - - const firstBeacon: Beacon = { - timestamp: 123456789, - context: context, - payload: payload, - }; - - const firstMessage: Envelope = { - id: '123', - message: JSON.stringify(firstBeacon), - }; - - await channel.publish(firstMessage); - - expect(publish).toHaveBeenCalled(); - - expect(JSON.parse(publish.mock.calls[0][0])).toStrictEqual({ - receiptId: '123', - originalTime: 123456789, - departureTime: now, - context: context, - payload: payload, - }); - - const secondBeacon: Beacon = { - token: 'some-token', - timestamp: 234567890, - context: context, - payload: payload, - }; - - const secondMessage: Envelope = { - id: '456', - message: JSON.stringify(secondBeacon), - }; - - await expect(channel.publish(secondMessage)).rejects.toThrow(); - }); - - it('should fail if an unexpected error occurs assigning a CID', async () => { - const error = new Error('Unexpected error'); - - const duplexChannel: DuplexChannel = { - close: jest.fn(), - publish: jest.fn(), - subscribe: jest.fn(), - unsubscribe: jest.fn(), - }; - const channel = new BeaconSocketChannel({ - channelFactory: (): DuplexChannel => duplexChannel, - tokenParameter: 'token', - trackerEndpointUrl: 'ws://localhost:8080', - cidParameter: 'clientId', - cidAssigner: {assignCid: jest.fn().mockRejectedValue(error)}, - }); - - const beacon: Beacon = { - token: 'some-token', - timestamp: 234567890, - context: context, - payload: payload, - }; - - const message: Envelope = { - id: '456', - message: JSON.stringify(beacon), - }; - - await expect(channel.publish(message)).rejects.toThrow(); - }); - - it('should allow subscribing and unsubscribing listeners', async () => { - const socketChannel = new SandboxChannel(); - const listener = jest.fn(); - const channel = new BeaconSocketChannel({ - channelFactory: (): SandboxChannel => socketChannel, - tokenParameter: 'token', - trackerEndpointUrl: 'ws://localhost:8080', - cidParameter: 'clientId', - cidAssigner: new FixedAssigner('123'), - }); - - channel.subscribe(listener); - - const beacon: Beacon = { - timestamp: 123456789, - context: context, - payload: payload, - }; - - const message: Envelope = { - id: '123', - message: JSON.stringify(beacon), - }; - - await channel.publish(message); - - socketChannel.notify(JSON.stringify({receiptId: '123'})); - - expect(listener).toHaveBeenNthCalledWith(1, '123'); - - channel.unsubscribe(listener); - - socketChannel.notify(JSON.stringify('123')); - - expect(listener).toHaveBeenCalledTimes(1); - }); - - it('should not notify listeners about invalid messages', async () => { - const socketChannel = new SandboxChannel(); - const listener = jest.fn(); - const channel = new BeaconSocketChannel({ - channelFactory: (): SandboxChannel => socketChannel, - tokenParameter: 'token', - trackerEndpointUrl: 'ws://localhost:8080', - cidParameter: 'clientId', - cidAssigner: new FixedAssigner('123'), - }); - - channel.subscribe(listener); - - const beacon: Beacon = { - timestamp: 123456789, - context: context, - payload: payload, - }; - - const message: Envelope = { - id: '123', - message: JSON.stringify(beacon), - }; - - await channel.publish(message); - - socketChannel.notify('invalid-json'); - - expect(listener).not.toHaveBeenCalled(); - }); - - it('should be able to be closed even if never used', async () => { - const socketChannel = new SandboxChannel(); - const channel = new BeaconSocketChannel({ - channelFactory: (): SandboxChannel => socketChannel, - tokenParameter: 'token', - trackerEndpointUrl: 'ws://localhost:8080', - cidParameter: 'clientId', - cidAssigner: new FixedAssigner('123'), - }); - - await expect(channel.close()).resolves.toBeUndefined(); - }); - - it('should close the socket channel on close', async () => { - const socketChannel = new SandboxChannel(); - const channel = new BeaconSocketChannel({ - channelFactory: (): SandboxChannel => socketChannel, - tokenParameter: 'token', - trackerEndpointUrl: 'ws://localhost:8080', - cidParameter: 'clientId', - cidAssigner: new FixedAssigner('123'), - }); - - const beacon: Beacon = { - timestamp: 123456789, - context: context, - payload: payload, - }; - - const message: Envelope = { - id: '123', - message: JSON.stringify(beacon), - }; - - await channel.publish(message).then(() => channel.close()); - - expect(socketChannel.isClosed()).toBeTruthy(); - }); -}); diff --git a/test/channel/httpBeaconChannel.test.ts b/test/channel/httpBeaconChannel.test.ts new file mode 100644 index 00000000..f2e2c2d3 --- /dev/null +++ b/test/channel/httpBeaconChannel.test.ts @@ -0,0 +1,289 @@ +import * as fetchMock from 'fetch-mock'; +import {HttpBeaconChannel} from '../../src/channel'; +import {Logger} from '../../src/logging'; +import {FixedAssigner} from '../../src/cid'; +import {Beacon} from '../../src/trackingEvents'; +import {Token} from '../../src/token'; + +describe('An HTTP beacon channel', () => { + beforeEach(() => { + fetchMock.reset(); + jest.clearAllMocks(); + jest.clearAllTimers(); + jest.useRealTimers(); + }); + + const appId = '00000000-0000-0000-0000-000000000000'; + const clientId = '00000000-0000-0000-0000-000000000001'; + const tabId = '00000000-0000-0000-0000-000000000002'; + const cidAssigner = new FixedAssigner(clientId); + + const logger: Logger = { + debug: jest.fn(), + warn: jest.fn(), + info: jest.fn(), + error: jest.fn(), + }; + + console.log(logger); + + const endpointUrl = 'http://api.croct.io/web/client/track'; + + it('should send a beacon to the specified URL', async () => { + fetchMock.mock(endpointUrl, 200); + + const channel = new HttpBeaconChannel({ + appId: appId, + endpointUrl: endpointUrl, + cidAssigner: cidAssigner, + }); + + jest.useFakeTimers({now: 2}); + + const beacon: Beacon = { + context: { + tabId: tabId, + url: 'http://example.com', + }, + payload: { + type: 'nothingChanged', + sinceTime: 0, + }, + token: Token.issue(appId).toString(), + timestamp: 1, + }; + + const listener = jest.fn(); + + channel.subscribe(listener); + + const receiptId = 'receipt-id'; + + const promise = channel.publish({ + id: receiptId, + message: JSON.stringify(beacon), + }); + + await expect(promise).resolves.toBeUndefined(); + + expect(listener).toHaveBeenCalledWith(receiptId); + + const calls = fetchMock.calls(); + + expect(calls).toHaveLength(1); + + const lastCall = calls[0] as fetchMock.MockCall; + const lastRequest = lastCall[1] as fetchMock.MockRequest; + + expect(lastCall[0]).toBe(endpointUrl); + + const {timestamp: originalTime, token, ...expectedBeacon} = beacon; + + expect(lastRequest.headers).toEqual({ + 'X-Application-Id': appId, + 'X-Client-Id': clientId, + 'X-Token': token, + }); + + expect(JSON.parse(lastRequest.body as string)).toEqual({ + ...expectedBeacon, + originalTime: originalTime, + departureTime: 2, + }); + }); + + it('should not send the token header if the token is not provided', async () => { + fetchMock.mock(endpointUrl, 200); + + const channel = new HttpBeaconChannel({ + appId: appId, + endpointUrl: endpointUrl, + cidAssigner: cidAssigner, + }); + + jest.useFakeTimers({now: 2}); + + const beacon: Beacon = { + context: { + tabId: tabId, + url: 'http://example.com', + }, + payload: { + type: 'nothingChanged', + sinceTime: 0, + }, + timestamp: 1, + }; + + const promise = channel.publish({ + id: 'receipt-id', + message: JSON.stringify(beacon), + }); + + await expect(promise).resolves.toBeUndefined(); + + const lastRequest = fetchMock.lastCall(endpointUrl)?.[1] as fetchMock.MockRequest; + + expect(lastRequest).not.toBeUndefined(); + + expect(lastRequest.headers).not.toContainKey('X-Token'); + }); + + it('should reject the promise if the response status is not OK', async () => { + fetchMock.mock(endpointUrl, 500); + + const channel = new HttpBeaconChannel({ + appId: appId, + endpointUrl: endpointUrl, + cidAssigner: cidAssigner, + logger: logger, + }); + + const listener = jest.fn(); + + channel.subscribe(listener); + + const promise = channel.publish({ + id: 'receipt-id', + message: JSON.stringify({ + context: { + tabId: tabId, + url: 'http://example.com', + }, + payload: { + type: 'nothingChanged', + sinceTime: 0, + }, + timestamp: 1, + }), + }); + + await expect(promise).rejects.toThrow('Internal Server Error'); + + expect(listener).not.toHaveBeenCalled(); + + expect(logger.error).toHaveBeenCalledWith('Failed to publish beacon: Internal Server Error'); + }); + + it.each([ + [403, 'API usage limit exceeded'], + [401, 'Invalid token'], + ])('should log an error and resolve the promise if the response status is %i', async (status, title) => { + fetchMock.mock(endpointUrl, { + status: status, + body: JSON.stringify({ + type: 'https://croct.help/api/event-tracker#error', + title: title, + status: status, + }), + }); + + const channel = new HttpBeaconChannel({ + appId: appId, + endpointUrl: endpointUrl, + cidAssigner: cidAssigner, + logger: logger, + }); + + const listener = jest.fn(); + + channel.subscribe(listener); + + const receiptId = 'receipt-id'; + + const promise = channel.publish({ + id: receiptId, + message: JSON.stringify({ + context: { + tabId: tabId, + url: 'http://example.com', + }, + payload: { + type: 'nothingChanged', + sinceTime: 0, + }, + timestamp: 1, + }), + }); + + await expect(promise).resolves.toBeUndefined(); + + expect(listener).toHaveBeenCalledWith(receiptId); + + expect(logger.error).toHaveBeenCalledWith(`Beacon rejected with non-retryable status: ${title}`); + }); + + it('should not notify listeners that have been unsubscribed', async () => { + fetchMock.mock(endpointUrl, 200); + + const channel = new HttpBeaconChannel({ + appId: appId, + endpointUrl: endpointUrl, + cidAssigner: cidAssigner, + }); + + const beacon: Beacon = { + context: { + tabId: tabId, + url: 'http://example.com', + }, + payload: { + type: 'nothingChanged', + sinceTime: 0, + }, + timestamp: 1, + }; + + const listener = jest.fn(); + + channel.subscribe(listener); + + channel.unsubscribe(listener); + + const promise = channel.publish({ + id: 'receipt-id', + message: JSON.stringify(beacon), + }); + + await expect(promise).resolves.toBeUndefined(); + + expect(listener).not.toHaveBeenCalled(); + }); + + it('should close the channel', async () => { + const channel = new HttpBeaconChannel({ + appId: appId, + endpointUrl: endpointUrl, + cidAssigner: cidAssigner, + }); + + const beacon: Beacon = { + context: { + tabId: tabId, + url: 'http://example.com', + }, + payload: { + type: 'nothingChanged', + sinceTime: 0, + }, + timestamp: 1, + }; + + const listener = jest.fn(); + + channel.subscribe(listener); + + channel.close(); + + const promise = channel.publish({ + id: 'receipt-id', + message: JSON.stringify(beacon), + }); + + await expect(promise).rejects.toThrow('Channel is closed'); + + expect(listener).not.toHaveBeenCalled(); + + expect(fetchMock.calls()).toHaveLength(0); + }); +}); diff --git a/test/channel/socketChannel.test.ts b/test/channel/socketChannel.test.ts deleted file mode 100644 index cd4fc8ea..00000000 --- a/test/channel/socketChannel.test.ts +++ /dev/null @@ -1,233 +0,0 @@ -import {WS} from 'jest-websocket-mock'; -import {SocketChannel} from '../../src/channel'; -import {Logger} from '../../src/logging'; - -describe('A socket channel', () => { - const url = 'ws://localhost:8080'; - - afterEach(() => { - WS.clean(); - jest.restoreAllMocks(); - }); - - it('should publish messages in order', async () => { - const server = new WS(url); - const channel = new SocketChannel({url: url}); - - await channel.publish('foo'); - await expect(server).toReceiveMessage('foo'); - - await channel.publish('bar'); - await expect(server).toReceiveMessage('bar'); - - expect(server).toHaveReceivedMessages(['foo', 'bar']); - }); - - it('should fail to publish messages if the connection is closed', async () => { - const server = new WS(url); - const channel = new SocketChannel({url: url}); - - await channel.publish('open connection'); - await server.connected; - - await channel.close(); - - await expect(channel.publish('bar')).rejects.toThrow('Channel has been closed.'); - }); - - it('should fail to publish messages if an error occurs in the meanwhile', async () => { - const server = new WS(url, {verifyClient: (): boolean => false}); - - const channel = new SocketChannel({ - url: url, - closeTimeout: 0, - connectionTimeout: 0, - }); - - await expect(channel.publish('foo')).rejects.toThrow(); - - await server.closed; - }); - - it('should reconnect if the connection cannot be established', async () => { - let attempt = 0; - - const server = new WS(url, { - verifyClient: (): boolean => attempt++ > 0, - }); - - const channel = new SocketChannel({url: url}); - - await expect(channel.publish('foo')).rejects.toThrow(); - await expect(channel.publish('bar')).resolves.toBeUndefined(); - - await server.connected; - }); - - it('should reconnect when receiving a message after the connection is closed', async () => { - const server = new WS(url); - const channel = new SocketChannel({url: url}); - - await expect(channel.publish('foo')).resolves.toBeUndefined(); - - const connection = await server.connected; - - connection.close({ - code: 1011, - reason: 'Server error', - wasClean: true, - }); - - await server.closed; - - await expect(channel.publish('bar')).resolves.toBeUndefined(); - }); - - it('should allow to subscribe and unsubscribe listeners', async () => { - const server = new WS(url); - const channel = new SocketChannel({url: url}); - const listener = jest.fn(); - - channel.subscribe(listener); - - expect(listener).not.toHaveBeenCalled(); - - await channel.publish('open connection'); - await server.connected; - - server.send('foo'); - - expect(listener).toHaveBeenCalledWith('foo'); - - channel.unsubscribe(listener); - - server.send('bar'); - - expect(listener).not.toHaveBeenCalledWith('bar'); - expect(listener).toHaveBeenCalledTimes(1); - }); - - it('should determine whether it is connected to the server or not', async () => { - const server = new WS(url); - const channel = new SocketChannel({url: url}); - - await expect(channel.connected).resolves.toBeFalsy(); - - await channel.publish('open connection'); - const connection = await server.connected; - - expect(connection.readyState).toBe(WebSocket.OPEN); - await expect(channel.connected).resolves.toBeTruthy(); - }); - - it('should configure the connection to use binary data type', async () => { - const server = new WS(url); - const channel = new SocketChannel({url: url, binaryType: 'blob'}); - - await expect(channel.connected).resolves.toBeFalsy(); - - await channel.publish('open connection'); - const connection = await server.connected; - - expect(connection.binaryType).toBe('blob'); - }); - - it('should allow to close the web socket connection', async () => { - const server = new WS(url); - const channel = new SocketChannel({url: url}); - - await channel.publish('open connection'); - - const connection = await server.connected; - - expect(connection.readyState).toBe(WebSocket.OPEN); - - await channel.close(); - - expect(connection.readyState).toBe(WebSocket.CLOSED); - }); - - it('should be able to close the channel even before establishing a connection', async () => { - const channel = new SocketChannel({url: url}); - - await expect(channel.close()).resolves.toBeUndefined(); - }); - - it('should close the web socket connection if timeout is reached', async () => { - const close = jest.fn(); - - window.WebSocket = class MockedSocket extends WebSocket { - public constructor() { - super('ws://foo'); - } - - public on(): void { - // ignore - } - - public addEventListener(): void { - // ignore - } - - public get onopen(): jest.Mock { - return jest.fn(); - } - - public get onmessage(): jest.Mock { - return jest.fn(); - } - - public get onclose(): jest.Mock { - return jest.fn(); - } - - public get onerror(): jest.Mock { - return jest.fn(); - } - - public close(code?: number, reason?: string): void { - close(code, reason); - } - }; - - const channel = new SocketChannel({url: url, connectionTimeout: 100}); - - await expect(channel.publish('timeout')).rejects.toThrow('Maximum connection timeout reached.'); - - expect(close).toHaveBeenCalledWith(1000, 'Maximum connection timeout reached.'); - }); - - it('should abort closing the channel if the timeout is reached', async () => { - const server = new WS(url); - const channel = new SocketChannel({url: url, closeTimeout: 0}); - - await channel.publish('open connection'); - await server.connected; - - await expect(channel.close()).rejects.toThrow('Maximum close timeout reached.'); - }); - - it('should close connection with error', async () => { - const channel = new SocketChannel({url: url}); - - await expect(channel.publish('open connection')).rejects.toThrow(); - - await expect(channel.close()).resolves.toBeUndefined(); - }); - - it('should close the connection if an error occurs', async () => { - const logger: Logger = { - debug: jest.fn(), - warn: jest.fn(), - info: jest.fn(), - error: jest.fn(), - }; - const server = new WS(url); - const channel = new SocketChannel({url: url, logger: logger}); - - server.error(); - - await expect(channel.publish('foo')).rejects.toThrow('Connection has been closed, reason'); - await expect(logger.error).toHaveBeenCalledWith('Connection error.'); - }); -}); diff --git a/test/container.test.ts b/test/container.test.ts index 875dcf1d..b432ee0e 100644 --- a/test/container.test.ts +++ b/test/container.test.ts @@ -1,4 +1,3 @@ -import {WS} from 'jest-websocket-mock'; import * as fetchMock from 'fetch-mock'; import {Configuration, Container, DependencyResolver} from '../src/container'; import {NullLogger, Logger} from '../src/logging'; @@ -10,17 +9,15 @@ import {TrackingEventProcessor} from '../src/tracker'; describe('A container', () => { beforeEach(() => { localStorage.clear(); + sessionStorage.clear(); for (const cookie of document.cookie.split(';')) { const [name] = cookie.split('='); document.cookie = `${name}=; Max-Age=0`; } - }); - afterEach(() => { jest.resetAllMocks(); - WS.clean(); fetchMock.reset(); }); @@ -34,7 +31,7 @@ describe('A container', () => { cidAssignerEndpointUrl: 'https://localtest/cid', contentBaseEndpointUrl: 'https://localtest/content', evaluationBaseEndpointUrl: 'https://localtest/evaluate', - trackerEndpointUrl: 'wss://localtest/connect', + trackerEndpointUrl: 'https://localtest/track', }; it('should provide its configuration', () => { @@ -265,6 +262,22 @@ describe('A container', () => { response: '123', }); + let attempts = 0; + + fetchMock.mock({ + method: 'POST', + matcher: (url: string) => url === configuration.trackerEndpointUrl && attempts++ === 0, + response: { + status: 500, + }, + }); + + fetchMock.mock({ + method: 'POST', + matcher: (url: string) => url === configuration.trackerEndpointUrl && attempts > 0, + response: 200, + }); + let container = new Container(configuration); let tracker = container.getTracker(); @@ -275,17 +288,19 @@ describe('A container', () => { const promise = tracker.track(payload); + await expect(new Promise(resolve => { setTimeout(resolve, 5); })).resolves.toBeUndefined(); + await container.dispose(); await expect(promise).rejects.toThrow(); - const server = new WS(`${configuration.trackerEndpointUrl}/${configuration.appId}`, {jsonProtocol: true}); + expect(fetchMock.calls(configuration.trackerEndpointUrl)).toHaveLength(1); container = new Container(configuration); - tracker = container.getTracker(); - tracker.enable(); - expect(server).toReceiveMessage(expect.objectContaining({payload: payload})); + await expect(tracker.flushed).resolves.toBeUndefined(); + + expect(fetchMock.calls(configuration.trackerEndpointUrl)).toHaveLength(2); }); it.each([ diff --git a/test/sdk.test.ts b/test/sdk.test.ts index 4e4d92c6..95e0e326 100644 --- a/test/sdk.test.ts +++ b/test/sdk.test.ts @@ -1,6 +1,6 @@ -import {WS} from 'jest-websocket-mock'; import * as fetchMock from 'fetch-mock'; -import {Sdk, Configuration, VERSION} from '../src'; +import {MockMatcher} from 'fetch-mock'; +import {Sdk, Configuration} from '../src'; import {NullLogger, Logger} from '../src/logging'; import {Token} from '../src/token'; import {TabEventEmulator} from './utils/tabEventEmulator'; @@ -35,33 +35,12 @@ describe('A SDK', () => { cookie: {}, }; - const websocketEndpoint = `${configuration.baseEndpointUrl.replace(/^http/i, 'ws')}/client/web/connect`; - - // Mock Socket does not support query strings: - // https://github.com/thoov/mock-socket/pull/231 - function creatWebSocketMock(endpoint: string): WS { - const ws = new WS(endpoint, {jsonProtocol: true}); - - window.WebSocket = class WebSocket extends window.WebSocket { - public constructor(originalUrl: string) { - const url = new URL(originalUrl); - - url.search = ''; - - super(url.toString()); - } - }; - - return ws; - } - beforeEach(() => { tabEventEmulator.registerListeners(); }); afterEach(() => { jest.clearAllMocks(); - WS.clean(); tabEventEmulator.reset(); fetchMock.reset(); localStorage.clear(); @@ -278,7 +257,7 @@ describe('A SDK', () => { [configuration.baseEndpointUrl, `${configuration.baseEndpointUrl}/client/web/cid`], ])( 'should configure the CID assigner with the base endpoint', - async (baseEndpoint: string|undefined, expectedEndpoint: string) => { + async (baseEndpoint: string | undefined, expectedEndpoint: string) => { fetchMock.mock({ method: 'GET', matcher: expectedEndpoint, @@ -299,98 +278,97 @@ describe('A SDK', () => { ); it('should ensure that events are delivered one at a time and in order', async () => { + const now = Date.now(); + + function createMatcher(index: number, negated: boolean = false): MockMatcher { + return (url: string, {body}: fetchMock.MockRequest) => { + if (url !== `${configuration.baseEndpointUrl}/client/web/track`) { + return false; + } + + const {payload: {sinceTime}} = JSON.parse(body as string); + + return negated ? sinceTime !== now + index : sinceTime === now + index; + }; + } + fetchMock.mock({ method: 'GET', matcher: configuration.cidAssignerEndpointUrl, response: '123', }); - const server = creatWebSocketMock(`${websocketEndpoint}/${configuration.appId}`); - const receiptIds: string[] = []; - - server.on('connection', socket => { - socket.on('message', message => { - const {receiptId} = JSON.parse(message as unknown as string); + fetchMock.mock({ + method: 'POST', + matcher: createMatcher(1), + // Add a delay to ensure the second message is sent before the first one is processed + delay: 200, + response: { + ok: true, + }, + }); - receiptIds.push(receiptId); - }); + fetchMock.mock({ + method: 'POST', + matcher: createMatcher(1, true), + response: { + ok: true, + }, }); const sdk = Sdk.init(configuration); const firstEvent: BeaconPayload = { type: 'nothingChanged', - sinceTime: Date.now() + 1, + sinceTime: now + 1, }; const firstPromise = sdk.tracker.track(firstEvent); const secondEvent: BeaconPayload = { type: 'nothingChanged', - sinceTime: Date.now() + 1, + sinceTime: now + 2, }; const secondPromise = sdk.tracker.track(secondEvent); - await expect(server).toReceiveMessage( - expect.objectContaining({ - payload: firstEvent, - }), - ); - - // Wait a few milliseconds more to ensure no other message was sent - await new Promise(resolve => { window.setTimeout(resolve, 30); }); - - expect(receiptIds.length).toBe(1); + await expect(Promise.all([firstPromise, secondPromise])).resolves.toEqual([firstEvent, secondEvent]); - server.send({ - receiptId: receiptIds[0], - violations: [], - }); + const calls = fetchMock.calls(); - await expect(firstPromise).resolves.toBe(firstEvent); + expect(calls).toHaveLength(2); - await expect(server).toReceiveMessage( - expect.objectContaining({ - payload: secondEvent, - }), - ); + const firstCall = calls[0][1] as fetchMock.MockRequest; - expect(receiptIds.length).toBe(2); + expect(JSON.parse(firstCall.body as string)).toEqual(expect.objectContaining({ + payload: firstEvent, + })); - server.send({ - receiptId: receiptIds[1], - violations: [], - }); + const secondCall = calls[1][1] as fetchMock.MockRequest; - await expect(secondPromise).resolves.toBe(secondEvent); + expect(JSON.parse(secondCall.body as string)).toEqual(expect.objectContaining({ + payload: secondEvent, + })); }); it.each([ - [undefined, `${BASE_ENDPOINT_URL.replace(/^http/i, 'ws')}/client/web/connect`], - [configuration.baseEndpointUrl, websocketEndpoint], + [undefined, `${BASE_ENDPOINT_URL}/client/web/track`], + [configuration.baseEndpointUrl, `${configuration.baseEndpointUrl}/client/web/track`], ])( 'should configure the tracker with the specified base endpoint', - async (baseEndpoint: string|undefined, expectedEndpoint: string) => { + async (baseEndpoint: string | undefined, expectedEndpoint: string) => { fetchMock.mock({ method: 'GET', matcher: configuration.cidAssignerEndpointUrl, response: '123', }); - const server = creatWebSocketMock( - `${expectedEndpoint}/${configuration.appId}`, - ); - - server.on('connection', socket => { - socket.on('message', message => { - const {receiptId} = JSON.parse(message as unknown as string); - - server.send({ - receiptId: receiptId, - violations: [], - }); - }); + fetchMock.mock({ + method: 'POST', + matcher: expectedEndpoint, + response: { + ok: true, + }, }); const metaName = 'foo'; @@ -415,16 +393,23 @@ describe('A SDK', () => { await expect(promise).resolves.toEqual(event); - await expect(server).toReceiveMessage(expect.objectContaining({ - receiptId: expect.stringMatching(/^\d+$/), - originalTime: expect.any(Number), - departureTime: expect.any(Number), - context: expect.objectContaining({ - metadata: { - sdkVersion: VERSION, - [`custom_${metaName}`]: metaValue, - }, - }), + const calls = fetchMock.calls(); + + expect(calls).toHaveLength(1); + + const lastCall = calls[0]; + + expect(lastCall[0]).toEqual(expectedEndpoint); + + const request = lastCall[1] as fetchMock.MockRequest; + + expect(request.headers).toEqual(expect.objectContaining({ + 'X-Application-Id': configuration.appId, + 'X-Client-Id': 'e6a133ffd3d2410681403d5e1bd95505', + })); + + expect(JSON.parse(request.body as string)).toEqual(expect.objectContaining({ + payload: event, })); }, ); @@ -434,7 +419,7 @@ describe('A SDK', () => { [configuration.baseEndpointUrl, configuration.baseEndpointUrl], ])( 'should configure the evaluator', - async (baseEndpoint: string|undefined, expectedEndpoint: string) => { + async (baseEndpoint: string | undefined, expectedEndpoint: string) => { const query = '1 + 2'; const result = 3; @@ -471,7 +456,7 @@ describe('A SDK', () => { [configuration.baseEndpointUrl, configuration.baseEndpointUrl], ])( 'should configure the content fetcher', - async (baseEndpoint: string|undefined, expectedEndpoint: string) => { + async (baseEndpoint: string | undefined, expectedEndpoint: string) => { const slotId = 'home-banner'; const result: FetchResponse = { content: { @@ -577,8 +562,6 @@ describe('A SDK', () => { response: '123', }); - const server = creatWebSocketMock(`${websocketEndpoint}/${configuration.appId}`); - const log = jest.fn(); const sdk = Sdk.init({ @@ -601,11 +584,8 @@ describe('A SDK', () => { // suppress error; }); - const connection = await server.connected; - await expect(sdk.close()).resolves.toBeUndefined(); - expect(connection.readyState).toBe(WebSocket.CLOSED); expect(tracker.isSuspended()).toBe(true); expect(log).toHaveBeenLastCalledWith('[Croct] SDK closed.'); From 5d3bde37aea140bd5f97ca52f5b0f7ff29aeb52a Mon Sep 17 00:00:00 2001 From: Marcos Passos Date: Fri, 2 Aug 2024 12:05:01 -0600 Subject: [PATCH 02/20] Fix persistent queue overriding issue --- src/container.ts | 2 +- src/queue/persistentQueue.ts | 33 ++++++++++-------------------- test/queue/persistentQueue.test.ts | 2 +- 3 files changed, 13 insertions(+), 24 deletions(-) diff --git a/src/container.ts b/src/container.ts index 836243df..461b6574 100644 --- a/src/container.ts +++ b/src/container.ts @@ -236,7 +236,7 @@ export class Container { logger: channelLogger, }), retryPolicy: new BackoffPolicy({ - minRetryDelay: 3000, // 1 second + minRetryDelay: 3000, // 3 second maxRetryDelay: 60 * 1000, // 60 seconds backoffFactor: 1.5, // 1.5 ^ attempt backoffJitter: 1, // add randomness diff --git a/src/queue/persistentQueue.ts b/src/queue/persistentQueue.ts index c62ecff9..4203e4e0 100644 --- a/src/queue/persistentQueue.ts +++ b/src/queue/persistentQueue.ts @@ -1,8 +1,6 @@ import {Queue} from './queue'; export class PersistentQueue implements Queue { - private cache: T[]; - private readonly storage: Storage; private readonly key: string; @@ -17,7 +15,7 @@ export class PersistentQueue implements Queue { } public getCapacity(): number { - return Infinity; + return Number.MAX_SAFE_INTEGER; } public isEmpty(): boolean { @@ -29,9 +27,7 @@ export class PersistentQueue implements Queue { } public push(value: T): void { - this.queue.push(value); - - this.flush(); + this.save([...this.queue, value]); } public peek(): T | null { @@ -45,30 +41,19 @@ export class PersistentQueue implements Queue { } public shift(): T { - const value = this.queue.shift(); + const queue = [...this.queue]; + const value = queue.shift(); if (value === undefined) { throw new Error('The queue is empty.'); } - this.flush(); + this.save(queue); return value; } - private get queue(): T[] { - if (this.cache === undefined) { - this.cache = this.load(); - } - - return this.cache; - } - - private flush(): void { - this.storage.setItem(this.key, JSON.stringify(this.cache ?? [])); - } - - private load(): T[] { + private get queue(): readonly T[] { const data = this.storage.getItem(this.key); if (data === null) { @@ -77,8 +62,12 @@ export class PersistentQueue implements Queue { try { return JSON.parse(data); - } catch (error) { + } catch { return []; } } + + private save(data: T[]): void { + this.storage.setItem(this.key, JSON.stringify(data)); + } } diff --git a/test/queue/persistentQueue.test.ts b/test/queue/persistentQueue.test.ts index e04e96ed..e89eb4da 100644 --- a/test/queue/persistentQueue.test.ts +++ b/test/queue/persistentQueue.test.ts @@ -14,7 +14,7 @@ describe('A persistent queue', () => { it('should have unlimited capacity', () => { const queue = new PersistentQueue(new DumbStorage()); - expect(queue.getCapacity()).toBe(Infinity); + expect(queue.getCapacity()).toBe(Number.MAX_SAFE_INTEGER); }); it('should determine whether the queue is empty', () => { From 1c5416a6baa69dc0fdfcb206e8a7d8d5e3d9d605 Mon Sep 17 00:00:00 2001 From: Marcos Passos Date: Fri, 2 Aug 2024 14:20:45 -0600 Subject: [PATCH 03/20] Update test/channel/httpBeaconChannel.test.ts Co-authored-by: Renan Oliveira --- test/channel/httpBeaconChannel.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/test/channel/httpBeaconChannel.test.ts b/test/channel/httpBeaconChannel.test.ts index f2e2c2d3..2f5f5080 100644 --- a/test/channel/httpBeaconChannel.test.ts +++ b/test/channel/httpBeaconChannel.test.ts @@ -25,7 +25,6 @@ describe('An HTTP beacon channel', () => { error: jest.fn(), }; - console.log(logger); const endpointUrl = 'http://api.croct.io/web/client/track'; From 13e331e4d04a045de13865765c107a5937af739d Mon Sep 17 00:00:00 2001 From: Marcos Passos Date: Fri, 2 Aug 2024 14:31:11 -0600 Subject: [PATCH 04/20] Add comment about the unclear wait --- test/container.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/container.test.ts b/test/container.test.ts index b432ee0e..40ccb361 100644 --- a/test/container.test.ts +++ b/test/container.test.ts @@ -288,7 +288,9 @@ describe('A container', () => { const promise = tracker.track(payload); - await expect(new Promise(resolve => { setTimeout(resolve, 5); })).resolves.toBeUndefined(); + // Add a delay to ensure the beacon is queued before disposing of the container. + // Otherwise, the beacon would be immediately rejected because the queue would be closed. + await new Promise(resolve => { setTimeout(resolve, 5); }); await container.dispose(); await expect(promise).rejects.toThrow(); From 19f61a98668b4482b48b039f1f6d45495c7758a0 Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Fri, 16 Aug 2024 11:20:53 -0300 Subject: [PATCH 05/20] Fix headers for new tracking --- src/channel/httpBeaconChannel.ts | 4 +++- test/channel/httpBeaconChannel.test.ts | 1 - 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/channel/httpBeaconChannel.ts b/src/channel/httpBeaconChannel.ts index 4ec18ca1..67ee6ea3 100644 --- a/src/channel/httpBeaconChannel.ts +++ b/src/channel/httpBeaconChannel.ts @@ -3,6 +3,7 @@ import {Envelope} from './guaranteedChannel'; import {Logger, NullLogger} from '../logging'; import {CidAssigner} from '../cid'; import {formatMessage} from '../error'; +import {CLIENT_LIBRARY} from '../constants'; export type Configuration = { appId: string, @@ -48,8 +49,9 @@ export class HttpBeaconChannel implements DuplexChannel { error: jest.fn(), }; - const endpointUrl = 'http://api.croct.io/web/client/track'; it('should send a beacon to the specified URL', async () => { From 60fb746eaa651080d93baa68be27c62ca465c05b Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Fri, 16 Aug 2024 12:35:33 -0300 Subject: [PATCH 06/20] Add preview build --- .github/workflows/pr-preview.yaml | 47 +++++++++++++++++++++++++++++++ 1 file changed, 47 insertions(+) create mode 100644 .github/workflows/pr-preview.yaml diff --git a/.github/workflows/pr-preview.yaml b/.github/workflows/pr-preview.yaml new file mode 100644 index 00000000..2c22e126 --- /dev/null +++ b/.github/workflows/pr-preview.yaml @@ -0,0 +1,47 @@ +name: PR preview + +on: + pull_request: + types: + - synchronize + - opened + +jobs: + preview: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-node@v4 + with: + node-version: 20 + + - name: Cache dependencies + id: cache-dependencies + uses: actions/cache@v4 + with: + path: node_modules + key: node_modules-${{ hashFiles('**/package-lock.json') }} + + - name: Install dependencies + if: steps.cache-dependencies.outputs.cache-hit != 'true' + run: npm ci + + - name: Build package + run: npm run build + + - name: Prepare release + run: |- + cp package.json LICENSE README.md build/ + cd build + find . -type f -path '*/*\.js.map' -exec sed -i -e "s~../src~src~" {} + + sed -i -e "s~\"version\": \"0.0.0-dev\"~\"version\": \"${GITHUB_REF##*/}\"~" package.json + sed -i -e "s~<@version@>~${GITHUB_REF##*/}~" constants.* + sed -i -e "s~<@baseEndpointUrl@>~${BASE_ENDPOINT}~" constants.* + sed -i -e "s~parseInt('<@maxQueryLength@>', 10)~${MAX_QUERY_LENGTH}~" constants.* + cp -r ../src src + + - name: Publish preview + run: |- + npx pkr-pr-new publish \ + --compact --comment=update \ + ./build From 40d68778fba093531a137d53b466a02d98e2911f Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Fri, 16 Aug 2024 12:36:24 -0300 Subject: [PATCH 07/20] Fix typo --- .github/workflows/pr-preview.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/pr-preview.yaml b/.github/workflows/pr-preview.yaml index 2c22e126..8182e1e4 100644 --- a/.github/workflows/pr-preview.yaml +++ b/.github/workflows/pr-preview.yaml @@ -42,6 +42,6 @@ jobs: - name: Publish preview run: |- - npx pkr-pr-new publish \ + npx pkg-pr-new publish \ --compact --comment=update \ ./build From 051f785444b972846d4edd5d6ced99e547e28fd3 Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Fri, 16 Aug 2024 12:44:06 -0300 Subject: [PATCH 08/20] Update preview job --- .github/workflows/pr-preview.yaml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/pr-preview.yaml b/.github/workflows/pr-preview.yaml index 8182e1e4..334e94cd 100644 --- a/.github/workflows/pr-preview.yaml +++ b/.github/workflows/pr-preview.yaml @@ -6,6 +6,10 @@ on: - synchronize - opened +env: + BASE_ENDPOINT: https://api.croct.io + MAX_QUERY_LENGTH: 500 + jobs: preview: runs-on: ubuntu-latest @@ -34,8 +38,7 @@ jobs: cp package.json LICENSE README.md build/ cd build find . -type f -path '*/*\.js.map' -exec sed -i -e "s~../src~src~" {} + - sed -i -e "s~\"version\": \"0.0.0-dev\"~\"version\": \"${GITHUB_REF##*/}\"~" package.json - sed -i -e "s~<@version@>~${GITHUB_REF##*/}~" constants.* + sed -i -e "s~<@version@>~0.0.0-dev~" constants.* sed -i -e "s~<@baseEndpointUrl@>~${BASE_ENDPOINT}~" constants.* sed -i -e "s~parseInt('<@maxQueryLength@>', 10)~${MAX_QUERY_LENGTH}~" constants.* cp -r ../src src From a143165c829cf09c41d26573a8b5c17ef91a97b0 Mon Sep 17 00:00:00 2001 From: Luiz Ferraz Date: Fri, 16 Aug 2024 12:57:45 -0300 Subject: [PATCH 09/20] Set content-type header --- src/channel/httpBeaconChannel.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/channel/httpBeaconChannel.ts b/src/channel/httpBeaconChannel.ts index 67ee6ea3..c3aaf8b1 100644 --- a/src/channel/httpBeaconChannel.ts +++ b/src/channel/httpBeaconChannel.ts @@ -52,6 +52,7 @@ export class HttpBeaconChannel implements DuplexChannel Date: Fri, 16 Aug 2024 16:39:43 -0300 Subject: [PATCH 10/20] Update tests --- test/channel/httpBeaconChannel.test.ts | 5 ++++- test/sdk.test.ts | 3 ++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/test/channel/httpBeaconChannel.test.ts b/test/channel/httpBeaconChannel.test.ts index 83b85cf4..65304d18 100644 --- a/test/channel/httpBeaconChannel.test.ts +++ b/test/channel/httpBeaconChannel.test.ts @@ -4,6 +4,7 @@ import {Logger} from '../../src/logging'; import {FixedAssigner} from '../../src/cid'; import {Beacon} from '../../src/trackingEvents'; import {Token} from '../../src/token'; +import {CLIENT_LIBRARY} from '../../src/constants'; describe('An HTTP beacon channel', () => { beforeEach(() => { @@ -78,9 +79,11 @@ describe('An HTTP beacon channel', () => { const {timestamp: originalTime, token, ...expectedBeacon} = beacon; expect(lastRequest.headers).toEqual({ - 'X-Application-Id': appId, 'X-Client-Id': clientId, 'X-Token': token, + 'X-App-Id': appId, + 'X-Client-Library': CLIENT_LIBRARY, + 'Content-Type': 'application/json', }); expect(JSON.parse(lastRequest.body as string)).toEqual({ diff --git a/test/sdk.test.ts b/test/sdk.test.ts index 95e0e326..0aa7f02d 100644 --- a/test/sdk.test.ts +++ b/test/sdk.test.ts @@ -404,8 +404,9 @@ describe('A SDK', () => { const request = lastCall[1] as fetchMock.MockRequest; expect(request.headers).toEqual(expect.objectContaining({ - 'X-Application-Id': configuration.appId, 'X-Client-Id': 'e6a133ffd3d2410681403d5e1bd95505', + 'X-App-Id': configuration.appId, + 'Content-Type': 'application/json', })); expect(JSON.parse(request.body as string)).toEqual(expect.objectContaining({ From 04f6c2898684915065ea8796dffb12678d4fbacf Mon Sep 17 00:00:00 2001 From: Denis Rossati Date: Fri, 16 Aug 2024 16:48:11 -0300 Subject: [PATCH 11/20] Fix comment typo --- src/container.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/container.ts b/src/container.ts index 461b6574..450dac3c 100644 --- a/src/container.ts +++ b/src/container.ts @@ -236,7 +236,7 @@ export class Container { logger: channelLogger, }), retryPolicy: new BackoffPolicy({ - minRetryDelay: 3000, // 3 second + minRetryDelay: 3000, // 3 seconds maxRetryDelay: 60 * 1000, // 60 seconds backoffFactor: 1.5, // 1.5 ^ attempt backoffJitter: 1, // add randomness From d9ce1fc9c9665aaa8165c5984190a6074a3f0760 Mon Sep 17 00:00:00 2001 From: Marcos Passos Date: Fri, 23 Aug 2024 13:28:51 -0600 Subject: [PATCH 12/20] Refactor te channels to explicitly signal retryability --- src/channel/channel.ts | 28 +++++++++++ src/channel/guaranteedChannel.ts | 8 +-- src/channel/httpBeaconChannel.ts | 26 +++++----- src/channel/queuedChannel.ts | 10 ++-- src/channel/retryChannel.ts | 14 ++++-- src/channel/sandboxChannel.ts | 6 ++- test/channel/channel.test.ts | 35 +++++++++++++ test/channel/guaranteedChannel.test.ts | 13 +++-- test/channel/httpBeaconChannel.test.ts | 70 ++++++++++++++++++++++---- test/channel/queuedChannel.test.ts | 24 ++++++--- test/channel/retryChannel.test.ts | 49 ++++++++++++++---- test/channel/sandboxChannel.test.ts | 7 ++- 12 files changed, 230 insertions(+), 60 deletions(-) create mode 100644 test/channel/channel.test.ts diff --git a/src/channel/channel.ts b/src/channel/channel.ts index 0145987b..1074420d 100644 --- a/src/channel/channel.ts +++ b/src/channel/channel.ts @@ -1,3 +1,31 @@ +import {formatMessage} from '../error'; + +export class MessageDeliveryError extends Error { + public readonly retryable: boolean; + + public constructor(message: string, retryable: boolean) { + super(message); + + this.retryable = retryable; + + Object.setPrototypeOf(this, MessageDeliveryError.prototype); + } + + public static fromCause(cause: any, retryable?: boolean): MessageDeliveryError { + if (cause instanceof MessageDeliveryError) { + return cause; + } + + const error = new MessageDeliveryError(formatMessage(cause), retryable ?? true); + + if (cause instanceof Error) { + error.stack = cause.stack; + } + + return error; + } +} + export interface Closeable { close(): Promise; } diff --git a/src/channel/guaranteedChannel.ts b/src/channel/guaranteedChannel.ts index 36772029..0d00a67c 100644 --- a/src/channel/guaranteedChannel.ts +++ b/src/channel/guaranteedChannel.ts @@ -1,5 +1,5 @@ import {Logger, NullLogger} from '../logging'; -import {DuplexChannel, OutputChannel} from './channel'; +import {DuplexChannel, MessageDeliveryError, OutputChannel} from './channel'; export type MessageStamper = { generate(message: M): S, @@ -49,7 +49,7 @@ export class GuaranteedChannel implements OutputChannel { public publish(message: M): Promise { if (this.closed) { - return Promise.reject(new Error('Channel is closed.')); + return Promise.reject(new MessageDeliveryError('Channel is closed.', false)); } return new Promise((resolve, reject): void => { @@ -99,7 +99,7 @@ export class GuaranteedChannel implements OutputChannel { () => { if (this.closed) { // Cancel delay immediately when the channel is closed - abort(new Error('Connection deliberately closed.')); + abort(new MessageDeliveryError('Connection deliberately closed.', false)); } }, 0, @@ -109,7 +109,7 @@ export class GuaranteedChannel implements OutputChannel { timeoutTimer = window.setTimeout( () => { - abort(new Error('Maximum confirmation time reached.')); + abort(new MessageDeliveryError('Maximum confirmation time reached.', true)); }, this.options.ackTimeout, ); diff --git a/src/channel/httpBeaconChannel.ts b/src/channel/httpBeaconChannel.ts index c3aaf8b1..a5fa5f8d 100644 --- a/src/channel/httpBeaconChannel.ts +++ b/src/channel/httpBeaconChannel.ts @@ -1,4 +1,4 @@ -import {ChannelListener, DuplexChannel} from './channel'; +import {ChannelListener, DuplexChannel, MessageDeliveryError} from './channel'; import {Envelope} from './guaranteedChannel'; import {Logger, NullLogger} from '../logging'; import {CidAssigner} from '../cid'; @@ -20,11 +20,6 @@ type ApiProblem = { }; export class HttpBeaconChannel implements DuplexChannel> { - private static readonly NON_RETRYABLE_STATUSES: ReadonlySet = new Set([ - 403, // API usage limit exceeded - 401, // Invalid token - ]); - private readonly configuration: Omit; private readonly logger: Logger; @@ -40,7 +35,7 @@ export class HttpBeaconChannel implements DuplexChannel): Promise { if (this.closed) { - return Promise.reject(new Error('Channel is closed')); + return Promise.reject(new MessageDeliveryError('Channel is closed', false)); } const {token, timestamp, context, payload} = JSON.parse(message); @@ -76,19 +71,17 @@ export class HttpBeaconChannel implements DuplexChannel { this.logger.error(`Failed to publish beacon: ${formatMessage(error)}`); - return Promise.reject(error); + return Promise.reject(MessageDeliveryError.fromCause(error, true)); }); } @@ -115,4 +108,9 @@ export class HttpBeaconChannel implements DuplexChannel= 500 || status === 429 || status === 408; + } } diff --git a/src/channel/queuedChannel.ts b/src/channel/queuedChannel.ts index 82218114..d2978196 100644 --- a/src/channel/queuedChannel.ts +++ b/src/channel/queuedChannel.ts @@ -1,4 +1,4 @@ -import {OutputChannel} from './channel'; +import {MessageDeliveryError, OutputChannel} from './channel'; import {Queue} from '../queue'; import {Logger, NullLogger} from '../logging'; @@ -29,19 +29,19 @@ export class QueuedChannel implements OutputChannel { public publish(message: T): Promise { if (this.closed) { - return Promise.reject(new Error('Channel is closed.')); + return Promise.reject(new MessageDeliveryError('Channel is closed.', false)); } if (this.queue.length() >= this.queue.getCapacity()) { this.logger.warn('The queue is full, message rejected.'); - return Promise.reject(new Error('The queue is full.')); + return Promise.reject(new MessageDeliveryError('The queue is full.', true)); } if (this.pending === undefined) { this.pending = this.queue.isEmpty() ? Promise.resolve() - : Promise.reject(new Error('The queue must be flushed.')); + : Promise.reject(new MessageDeliveryError('The queue must be flushed.', true)); } this.enqueue(message); @@ -71,7 +71,7 @@ export class QueuedChannel implements OutputChannel { private requeue(): Promise { if (this.closed) { - return Promise.reject(new Error('Channel is closed.')); + return Promise.reject(new MessageDeliveryError('Channel is closed.', false)); } this.pending = Promise.resolve(); diff --git a/src/channel/retryChannel.ts b/src/channel/retryChannel.ts index dfa560c4..1707b21c 100644 --- a/src/channel/retryChannel.ts +++ b/src/channel/retryChannel.ts @@ -1,4 +1,4 @@ -import {OutputChannel} from './channel'; +import {MessageDeliveryError, OutputChannel} from './channel'; import {Logger, NullLogger} from '../logging'; import {RetryPolicy} from '../retry'; @@ -25,7 +25,7 @@ export class RetryChannel implements OutputChannel { public publish(message: T): Promise { if (this.closed) { - return Promise.reject(new Error('The channel is closed.')); + return Promise.reject(new MessageDeliveryError('The channel is closed.', false)); } return this.channel @@ -34,11 +34,15 @@ export class RetryChannel implements OutputChannel { } public async retry(message: T, error: unknown): Promise { + if (error instanceof MessageDeliveryError && !error.retryable) { + throw error; + } + let attempt = 0; while (this.retryPolicy.shouldRetry(attempt, message, error)) { if (this.closed) { - throw new Error('Connection deliberately closed.'); + throw new MessageDeliveryError('Connection deliberately closed.', false); } const delay = this.retryPolicy.getDelay(attempt); @@ -55,7 +59,7 @@ export class RetryChannel implements OutputChannel { // Cancel delay immediately when the channel is closed window.clearInterval(closeWatcher); - reject(new Error('Connection deliberately closed.')); + reject(new MessageDeliveryError('Connection deliberately closed.', false)); } }, 0, @@ -79,7 +83,7 @@ export class RetryChannel implements OutputChannel { } } - throw new Error('Maximum retry attempts reached.'); + throw new MessageDeliveryError('Maximum retry attempts reached.', false); } public close(): Promise { diff --git a/src/channel/sandboxChannel.ts b/src/channel/sandboxChannel.ts index ffe7b779..43ab3139 100644 --- a/src/channel/sandboxChannel.ts +++ b/src/channel/sandboxChannel.ts @@ -1,4 +1,4 @@ -import {ChannelListener, DuplexChannel} from './channel'; +import {ChannelListener, DuplexChannel, MessageDeliveryError} from './channel'; export class SandboxChannel implements DuplexChannel { private readonly listeners: Array> = []; @@ -8,6 +8,10 @@ export class SandboxChannel implements DuplexChannel { private closed = false; public publish(message: O): Promise { + if (this.closed) { + return Promise.reject(new MessageDeliveryError('Channel is closed.', false)); + } + this.messages.push(message); return Promise.resolve(); diff --git a/test/channel/channel.test.ts b/test/channel/channel.test.ts new file mode 100644 index 00000000..a433de12 --- /dev/null +++ b/test/channel/channel.test.ts @@ -0,0 +1,35 @@ +import {MessageDeliveryError} from '../../src/channel'; + +describe('MessageDeliveryError', () => { + it('should initialize the error message and retryable flag', () => { + const error = new MessageDeliveryError('test', true); + + expect(error.message).toBe('test'); + expect(error.retryable).toBe(true); + }); + + it('should return the cause if it is a MessageDeliveryError', () => { + const cause = new MessageDeliveryError('test', true); + const error = MessageDeliveryError.fromCause(cause); + + expect(error).toBe(cause); + }); + + it('should initialize the error as retryable from arbitrary causes', () => { + const cause = new Error('test'); + const error = MessageDeliveryError.fromCause(cause); + + expect(error.message).toBe('Test'); + expect(error.retryable).toBe(true); + expect(error.stack).toBe(cause.stack); + }); + + it('should initialize the error using the retryable flag from arbitrary causes', () => { + const cause = new Error('Test'); + const error = MessageDeliveryError.fromCause(cause, false); + + expect(error.message).toBe('Test'); + expect(error.retryable).toBe(false); + expect(error.stack).toBe(cause.stack); + }); +}); diff --git a/test/channel/guaranteedChannel.test.ts b/test/channel/guaranteedChannel.test.ts index 05a08157..59aaa3cc 100644 --- a/test/channel/guaranteedChannel.test.ts +++ b/test/channel/guaranteedChannel.test.ts @@ -1,5 +1,5 @@ import {Envelope, GuaranteedChannel, MessageStamper, TimeStamper} from '../../src/channel/guaranteedChannel'; -import {SandboxChannel} from '../../src/channel'; +import {MessageDeliveryError, SandboxChannel} from '../../src/channel'; describe('A guaranteed channel', () => { let stamper: MessageStamper; @@ -54,7 +54,8 @@ describe('A guaranteed channel', () => { // Invalid acknowledge stamp sandboxChannel.notify('pong_stamp'); - await expect(promise).rejects.toThrow('Maximum confirmation time reached.'); + await expect(promise).rejects.toThrowWithMessage(MessageDeliveryError, 'Maximum confirmation time reached.'); + await expect(promise).rejects.toHaveProperty('retryable', true); }); it('should stop waiting for confirmation if the channel is closed in the meanwhile', async () => { @@ -64,7 +65,8 @@ describe('A guaranteed channel', () => { await channel.close(); - await expect(promise).rejects.toEqual(new Error('Connection deliberately closed.')); + await expect(promise).rejects.toThrowWithMessage(MessageDeliveryError, 'Connection deliberately closed.'); + await expect(promise).rejects.toHaveProperty('retryable', false); }); it('should close the output channel on close', async () => { @@ -76,7 +78,10 @@ describe('A guaranteed channel', () => { it('should fail to publish messages if the channel is closed', async () => { await channel.close(); - await expect(channel.publish('foo')).rejects.toEqual(new Error('Channel is closed.')); + const promise = channel.publish('foo'); + + await expect(promise).rejects.toThrowWithMessage(MessageDeliveryError, 'Channel is closed.'); + await expect(promise).rejects.toHaveProperty('retryable', false); }); }); diff --git a/test/channel/httpBeaconChannel.test.ts b/test/channel/httpBeaconChannel.test.ts index 65304d18..3aabc12b 100644 --- a/test/channel/httpBeaconChannel.test.ts +++ b/test/channel/httpBeaconChannel.test.ts @@ -1,5 +1,5 @@ import * as fetchMock from 'fetch-mock'; -import {HttpBeaconChannel} from '../../src/channel'; +import {HttpBeaconChannel, MessageDeliveryError} from '../../src/channel'; import {Logger} from '../../src/logging'; import {FixedAssigner} from '../../src/cid'; import {Beacon} from '../../src/trackingEvents'; @@ -159,7 +159,8 @@ describe('An HTTP beacon channel', () => { }), }); - await expect(promise).rejects.toThrow('Internal Server Error'); + await expect(promise).rejects.toThrowWithMessage(MessageDeliveryError, 'Internal Server Error'); + await expect(promise).rejects.toHaveProperty('retryable', true); expect(listener).not.toHaveBeenCalled(); @@ -167,9 +168,11 @@ describe('An HTTP beacon channel', () => { }); it.each([ - [403, 'API usage limit exceeded'], [401, 'Invalid token'], - ])('should log an error and resolve the promise if the response status is %i', async (status, title) => { + [403, 'Unallowed origin'], + [423, 'API usage limit exceeded'], + [402, 'Payment overdue'], + ])('should report a non-retryable error if the response status is %i', async (status, title) => { fetchMock.mock(endpointUrl, { status: status, body: JSON.stringify({ @@ -207,13 +210,63 @@ describe('An HTTP beacon channel', () => { }), }); - await expect(promise).resolves.toBeUndefined(); + await expect(promise).rejects.toThrowWithMessage(MessageDeliveryError, title); + await expect(promise).rejects.toHaveProperty('retryable', false); - expect(listener).toHaveBeenCalledWith(receiptId); + expect(listener).not.toHaveBeenCalled(); expect(logger.error).toHaveBeenCalledWith(`Beacon rejected with non-retryable status: ${title}`); }); + it.each([ + [429, 'Rate limit exceeded'], + [408, 'Request timeout'], + [503, 'Service unavailable'], + [504, 'Gateway timeout'], + ])('should report a retryable error if the response status is %i', async (status, title) => { + fetchMock.mock(endpointUrl, { + status: status, + body: JSON.stringify({ + type: 'https://croct.help/api/event-tracker#error', + title: title, + status: status, + }), + }); + + const channel = new HttpBeaconChannel({ + appId: appId, + endpointUrl: endpointUrl, + cidAssigner: cidAssigner, + logger: logger, + }); + + const listener = jest.fn(); + + channel.subscribe(listener); + + const receiptId = 'receipt-id'; + + const promise = channel.publish({ + id: receiptId, + message: JSON.stringify({ + context: { + tabId: tabId, + url: 'http://example.com', + }, + payload: { + type: 'nothingChanged', + sinceTime: 0, + }, + timestamp: 1, + }), + }); + + await expect(promise).rejects.toThrowWithMessage(MessageDeliveryError, title); + await expect(promise).rejects.toHaveProperty('retryable', true); + + expect(listener).not.toHaveBeenCalled(); + }); + it('should not notify listeners that have been unsubscribed', async () => { fetchMock.mock(endpointUrl, 200); @@ -281,9 +334,8 @@ describe('An HTTP beacon channel', () => { message: JSON.stringify(beacon), }); - await expect(promise).rejects.toThrow('Channel is closed'); - - expect(listener).not.toHaveBeenCalled(); + await expect(promise).rejects.toThrowWithMessage(MessageDeliveryError, 'Channel is closed'); + await expect(promise).rejects.toHaveProperty('retryable', false); expect(fetchMock.calls()).toHaveLength(0); }); diff --git a/test/channel/queuedChannel.test.ts b/test/channel/queuedChannel.test.ts index 2814f970..deae745a 100644 --- a/test/channel/queuedChannel.test.ts +++ b/test/channel/queuedChannel.test.ts @@ -1,5 +1,5 @@ import {InMemoryQueue, CapacityRestrictedQueue} from '../../src/queue'; -import {QueuedChannel, OutputChannel} from '../../src/channel'; +import {QueuedChannel, OutputChannel, MessageDeliveryError} from '../../src/channel'; describe('A queued channel', () => { afterEach(() => { @@ -11,7 +11,7 @@ describe('A queued channel', () => { close: jest.fn().mockResolvedValue(undefined), publish: jest.fn() .mockResolvedValueOnce(undefined) - .mockRejectedValueOnce(new Error('Rejected')) + .mockRejectedValueOnce(new MessageDeliveryError('Rejected', true)) .mockResolvedValue(undefined), }; const channel = new QueuedChannel(outputChannel, new InMemoryQueue('foo', 'bar')); @@ -89,7 +89,10 @@ describe('A queued channel', () => { await channel.close(); - await expect(channel.flush()).rejects.toEqual(new Error('Channel is closed.')); + const promise = channel.flush(); + + await expect(promise).rejects.toThrowWithMessage(MessageDeliveryError, 'Channel is closed.'); + await expect(promise).rejects.toHaveProperty('retryable', false); }); it('should fail to publish messages if the queue is full', async () => { @@ -99,7 +102,10 @@ describe('A queued channel', () => { }; const channel = new QueuedChannel(outputChannel, new CapacityRestrictedQueue(new InMemoryQueue('foo'), 1)); - await expect(channel.publish('bar')).rejects.toThrow('The queue is full.'); + const promise = channel.publish('bar'); + + await expect(promise).rejects.toThrowWithMessage(MessageDeliveryError, 'The queue is full.'); + await expect(promise).rejects.toHaveProperty('retryable', true); }); it('should fail to publish messages if the channel is closed', async () => { @@ -111,7 +117,10 @@ describe('A queued channel', () => { await channel.close(); - await expect(channel.publish('foo')).rejects.toEqual(new Error('Channel is closed.')); + const promise = channel.publish('foo'); + + await expect(promise).rejects.toThrowWithMessage(MessageDeliveryError, 'Channel is closed.'); + await expect(promise).rejects.toHaveProperty('retryable', false); }); it('should fail to publish messages if queue has pending messages', async () => { @@ -121,7 +130,10 @@ describe('A queued channel', () => { }; const channel = new QueuedChannel(outputChannel, new InMemoryQueue('foo')); - await expect(channel.publish('bar')).rejects.toEqual(expect.any(Error)); + const promise = channel.publish('bar'); + + await expect(promise).rejects.toThrowWithMessage(MessageDeliveryError, 'The queue must be flushed.'); + await expect(promise).rejects.toHaveProperty('retryable', true); await channel.flush(); diff --git a/test/channel/retryChannel.test.ts b/test/channel/retryChannel.test.ts index 1843054d..92b68dbd 100644 --- a/test/channel/retryChannel.test.ts +++ b/test/channel/retryChannel.test.ts @@ -1,4 +1,4 @@ -import {RetryChannel, OutputChannel} from '../../src/channel'; +import {RetryChannel, OutputChannel, MessageDeliveryError} from '../../src/channel'; import {MaxAttemptsPolicy} from '../../src/retry'; describe('A retry channel', () => { @@ -26,8 +26,8 @@ describe('A retry channel', () => { const outputChannel: OutputChannel = { close: jest.fn().mockResolvedValue(undefined), publish: jest.fn() - .mockRejectedValueOnce(new Error('Rejected')) - .mockRejectedValueOnce(new Error('Rejected')) + .mockRejectedValueOnce(new MessageDeliveryError('Rejected', true)) + .mockRejectedValueOnce(new MessageDeliveryError('Rejected', true)) .mockResolvedValue(undefined), }; const channel = new RetryChannel({ @@ -55,13 +55,16 @@ describe('A retry channel', () => { await channel.close(); - await expect(channel.publish('foo')).rejects.toEqual(new Error('The channel is closed.')); + const promise = channel.publish('foo'); + + await expect(promise).rejects.toThrowWithMessage(MessageDeliveryError, 'The channel is closed.'); + await expect(promise).rejects.toHaveProperty('retryable', false); }); it('should fail to publish a message if the channel is closed before retrying', async () => { const outputChannel: OutputChannel = { close: jest.fn().mockResolvedValue(undefined), - publish: jest.fn().mockRejectedValue(new Error('Rejected')), + publish: jest.fn().mockRejectedValue(new MessageDeliveryError('Rejected', true)), }; const channel = new RetryChannel({ channel: outputChannel, @@ -72,13 +75,14 @@ describe('A retry channel', () => { await channel.close(); - await expect(promise).rejects.toEqual(new Error('Connection deliberately closed.')); + await expect(promise).rejects.toThrowWithMessage(MessageDeliveryError, 'Connection deliberately closed.'); + await expect(promise).rejects.toHaveProperty('retryable', false); }); it('should fail to publish a message if the channel is closed while retrying', async () => { const outputChannel: OutputChannel = { close: jest.fn().mockResolvedValue(undefined), - publish: jest.fn().mockRejectedValue(new Error('Rejected')), + publish: jest.fn().mockRejectedValue(new MessageDeliveryError('Rejected', true)), }; const channel = new RetryChannel({ channel: outputChannel, @@ -91,27 +95,50 @@ describe('A retry channel', () => { await channel.close(); - await expect(promise).rejects.toEqual(new Error('Connection deliberately closed.')); + await expect(promise).rejects.toThrowWithMessage(MessageDeliveryError, 'Connection deliberately closed.'); + await expect(promise).rejects.toHaveProperty('retryable', false); }); it('should fail to publish a message if maximum retry attempts is reached', async () => { const outputChannel: OutputChannel = { close: jest.fn().mockResolvedValue(undefined), publish: jest.fn() - .mockRejectedValueOnce(new Error('Rejected')) - .mockRejectedValueOnce(new Error('Rejected')), + .mockRejectedValueOnce(new MessageDeliveryError('Rejected', true)) + .mockRejectedValueOnce(new MessageDeliveryError('Rejected', true)), }; const channel = new RetryChannel({ channel: outputChannel, retryPolicy: new MaxAttemptsPolicy(0, 1), }); - await expect(channel.publish('foo')).rejects.toEqual(new Error('Maximum retry attempts reached.')); + const promise = channel.publish('foo'); + + await expect(promise).rejects.toThrowWithMessage(MessageDeliveryError, 'Maximum retry attempts reached.'); + await expect(promise).rejects.toHaveProperty('retryable', false); + expect(outputChannel.publish).toHaveBeenNthCalledWith(1, 'foo'); expect(outputChannel.publish).toHaveBeenNthCalledWith(2, 'foo'); expect(outputChannel.publish).toHaveBeenCalledTimes(2); }); + it('should not retry if an error is not retryable', async () => { + const outputChannel: OutputChannel = { + close: jest.fn().mockResolvedValue(undefined), + publish: jest.fn().mockRejectedValue(new MessageDeliveryError('Rejected', false)), + }; + const channel = new RetryChannel({ + channel: outputChannel, + retryPolicy: new MaxAttemptsPolicy(0, 1), + }); + + const promise = channel.publish('foo'); + + await expect(promise).rejects.toThrowWithMessage(MessageDeliveryError, 'Rejected'); + await expect(promise).rejects.toHaveProperty('retryable', false); + + expect(outputChannel.publish).toHaveBeenCalledWith('foo'); + }); + it('should close the output channel on close', async () => { const outputChannel: OutputChannel = { close: jest.fn(), diff --git a/test/channel/sandboxChannel.test.ts b/test/channel/sandboxChannel.test.ts index e226ef5a..6120ccbf 100644 --- a/test/channel/sandboxChannel.test.ts +++ b/test/channel/sandboxChannel.test.ts @@ -1,4 +1,4 @@ -import {SandboxChannel} from '../../src/channel'; +import {MessageDeliveryError, SandboxChannel} from '../../src/channel'; describe('A sandbox channel', () => { afterEach(() => { @@ -45,5 +45,10 @@ describe('A sandbox channel', () => { await channel.close(); expect(channel.isClosed()).toBe(true); + + const promise = channel.publish('foo'); + + await expect(promise).rejects.toThrowWithMessage(MessageDeliveryError, 'Channel is closed.'); + await expect(promise).rejects.toHaveProperty('retryable', false); }); }); From 6a450c314c91fa917f157a4e6be3fb4630c5f054 Mon Sep 17 00:00:00 2001 From: Marcos Passos Date: Fri, 23 Aug 2024 14:05:30 -0600 Subject: [PATCH 13/20] Add named constructors for instantiating delivery errors --- src/channel/channel.ts | 16 ++++++++++------ src/channel/guaranteedChannel.ts | 6 +++--- src/channel/httpBeaconChannel.ts | 10 +++++++--- src/channel/queuedChannel.ts | 8 ++++---- src/channel/retryChannel.ts | 8 ++++---- src/channel/sandboxChannel.ts | 2 +- test/channel/channel.test.ts | 24 ++++++++++++------------ 7 files changed, 41 insertions(+), 33 deletions(-) diff --git a/src/channel/channel.ts b/src/channel/channel.ts index 1074420d..f8716442 100644 --- a/src/channel/channel.ts +++ b/src/channel/channel.ts @@ -3,7 +3,7 @@ import {formatMessage} from '../error'; export class MessageDeliveryError extends Error { public readonly retryable: boolean; - public constructor(message: string, retryable: boolean) { + private constructor(message: string, retryable: boolean) { super(message); this.retryable = retryable; @@ -11,12 +11,16 @@ export class MessageDeliveryError extends Error { Object.setPrototypeOf(this, MessageDeliveryError.prototype); } - public static fromCause(cause: any, retryable?: boolean): MessageDeliveryError { - if (cause instanceof MessageDeliveryError) { - return cause; - } + public static retryable(cause: unknown): MessageDeliveryError { + return MessageDeliveryError.fromCause(cause, true); + } + + public static nonRetryable(cause: unknown): MessageDeliveryError { + return MessageDeliveryError.fromCause(cause, false); + } - const error = new MessageDeliveryError(formatMessage(cause), retryable ?? true); + private static fromCause(cause: any, retryable: boolean): MessageDeliveryError { + const error = new MessageDeliveryError(formatMessage(cause), retryable); if (cause instanceof Error) { error.stack = cause.stack; diff --git a/src/channel/guaranteedChannel.ts b/src/channel/guaranteedChannel.ts index 0d00a67c..a2f264ca 100644 --- a/src/channel/guaranteedChannel.ts +++ b/src/channel/guaranteedChannel.ts @@ -49,7 +49,7 @@ export class GuaranteedChannel implements OutputChannel { public publish(message: M): Promise { if (this.closed) { - return Promise.reject(new MessageDeliveryError('Channel is closed.', false)); + return Promise.reject(MessageDeliveryError.nonRetryable('Channel is closed.')); } return new Promise((resolve, reject): void => { @@ -99,7 +99,7 @@ export class GuaranteedChannel implements OutputChannel { () => { if (this.closed) { // Cancel delay immediately when the channel is closed - abort(new MessageDeliveryError('Connection deliberately closed.', false)); + abort(MessageDeliveryError.nonRetryable('Connection deliberately closed.')); } }, 0, @@ -109,7 +109,7 @@ export class GuaranteedChannel implements OutputChannel { timeoutTimer = window.setTimeout( () => { - abort(new MessageDeliveryError('Maximum confirmation time reached.', true)); + abort(MessageDeliveryError.retryable('Maximum confirmation time reached.')); }, this.options.ackTimeout, ); diff --git a/src/channel/httpBeaconChannel.ts b/src/channel/httpBeaconChannel.ts index a5fa5f8d..3038e3e6 100644 --- a/src/channel/httpBeaconChannel.ts +++ b/src/channel/httpBeaconChannel.ts @@ -35,7 +35,7 @@ export class HttpBeaconChannel implements DuplexChannel): Promise { if (this.closed) { - return Promise.reject(new MessageDeliveryError('Channel is closed', false)); + return Promise.reject(MessageDeliveryError.nonRetryable('Channel is closed')); } const {token, timestamp, context, payload} = JSON.parse(message); @@ -77,11 +77,15 @@ export class HttpBeaconChannel implements DuplexChannel { this.logger.error(`Failed to publish beacon: ${formatMessage(error)}`); - return Promise.reject(MessageDeliveryError.fromCause(error, true)); + return Promise.reject(MessageDeliveryError.retryable(error)); }); } diff --git a/src/channel/queuedChannel.ts b/src/channel/queuedChannel.ts index d2978196..fd68a0d1 100644 --- a/src/channel/queuedChannel.ts +++ b/src/channel/queuedChannel.ts @@ -29,19 +29,19 @@ export class QueuedChannel implements OutputChannel { public publish(message: T): Promise { if (this.closed) { - return Promise.reject(new MessageDeliveryError('Channel is closed.', false)); + return Promise.reject(MessageDeliveryError.nonRetryable('Channel is closed.')); } if (this.queue.length() >= this.queue.getCapacity()) { this.logger.warn('The queue is full, message rejected.'); - return Promise.reject(new MessageDeliveryError('The queue is full.', true)); + return Promise.reject(MessageDeliveryError.retryable('The queue is full.')); } if (this.pending === undefined) { this.pending = this.queue.isEmpty() ? Promise.resolve() - : Promise.reject(new MessageDeliveryError('The queue must be flushed.', true)); + : Promise.reject(MessageDeliveryError.retryable('The queue must be flushed.')); } this.enqueue(message); @@ -71,7 +71,7 @@ export class QueuedChannel implements OutputChannel { private requeue(): Promise { if (this.closed) { - return Promise.reject(new MessageDeliveryError('Channel is closed.', false)); + return Promise.reject(MessageDeliveryError.nonRetryable('Channel is closed.')); } this.pending = Promise.resolve(); diff --git a/src/channel/retryChannel.ts b/src/channel/retryChannel.ts index 1707b21c..daec4b0c 100644 --- a/src/channel/retryChannel.ts +++ b/src/channel/retryChannel.ts @@ -25,7 +25,7 @@ export class RetryChannel implements OutputChannel { public publish(message: T): Promise { if (this.closed) { - return Promise.reject(new MessageDeliveryError('The channel is closed.', false)); + return Promise.reject(MessageDeliveryError.nonRetryable('The channel is closed.')); } return this.channel @@ -42,7 +42,7 @@ export class RetryChannel implements OutputChannel { while (this.retryPolicy.shouldRetry(attempt, message, error)) { if (this.closed) { - throw new MessageDeliveryError('Connection deliberately closed.', false); + throw MessageDeliveryError.nonRetryable('Connection deliberately closed.'); } const delay = this.retryPolicy.getDelay(attempt); @@ -59,7 +59,7 @@ export class RetryChannel implements OutputChannel { // Cancel delay immediately when the channel is closed window.clearInterval(closeWatcher); - reject(new MessageDeliveryError('Connection deliberately closed.', false)); + reject(MessageDeliveryError.nonRetryable('Connection deliberately closed.')); } }, 0, @@ -83,7 +83,7 @@ export class RetryChannel implements OutputChannel { } } - throw new MessageDeliveryError('Maximum retry attempts reached.', false); + throw MessageDeliveryError.nonRetryable('Maximum retry attempts reached.'); } public close(): Promise { diff --git a/src/channel/sandboxChannel.ts b/src/channel/sandboxChannel.ts index 43ab3139..0af40e32 100644 --- a/src/channel/sandboxChannel.ts +++ b/src/channel/sandboxChannel.ts @@ -9,7 +9,7 @@ export class SandboxChannel implements DuplexChannel { public publish(message: O): Promise { if (this.closed) { - return Promise.reject(new MessageDeliveryError('Channel is closed.', false)); + return Promise.reject(MessageDeliveryError.nonRetryable('Channel is closed.')); } this.messages.push(message); diff --git a/test/channel/channel.test.ts b/test/channel/channel.test.ts index a433de12..0cb5c314 100644 --- a/test/channel/channel.test.ts +++ b/test/channel/channel.test.ts @@ -1,32 +1,32 @@ import {MessageDeliveryError} from '../../src/channel'; describe('MessageDeliveryError', () => { - it('should initialize the error message and retryable flag', () => { - const error = new MessageDeliveryError('test', true); + it('should initialize a retryable error', () => { + const error = MessageDeliveryError.retryable('test'); - expect(error.message).toBe('test'); + expect(error.message).toBe('Test'); expect(error.retryable).toBe(true); }); - it('should return the cause if it is a MessageDeliveryError', () => { - const cause = new MessageDeliveryError('test', true); - const error = MessageDeliveryError.fromCause(cause); + it('should initialize a non-retryable error', () => { + const error = MessageDeliveryError.nonRetryable('test'); - expect(error).toBe(cause); + expect(error.message).toBe('Test'); + expect(error.retryable).toBe(false); }); - it('should initialize the error as retryable from arbitrary causes', () => { - const cause = new Error('test'); - const error = MessageDeliveryError.fromCause(cause); + it('should initialize a retryable error from another error', () => { + const cause = new Error('Test'); + const error = MessageDeliveryError.retryable(cause); expect(error.message).toBe('Test'); expect(error.retryable).toBe(true); expect(error.stack).toBe(cause.stack); }); - it('should initialize the error using the retryable flag from arbitrary causes', () => { + it('should initialize a non-retryable error from another error', () => { const cause = new Error('Test'); - const error = MessageDeliveryError.fromCause(cause, false); + const error = MessageDeliveryError.nonRetryable(cause); expect(error.message).toBe('Test'); expect(error.retryable).toBe(false); From 9e1c8f1259050b5b932c67cd8b7cbd414ada20b7 Mon Sep 17 00:00:00 2001 From: Marcos Passos Date: Fri, 23 Aug 2024 14:05:50 -0600 Subject: [PATCH 14/20] Update src/channel/httpBeaconChannel.ts Co-authored-by: Luiz Ferraz --- src/channel/httpBeaconChannel.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/channel/httpBeaconChannel.ts b/src/channel/httpBeaconChannel.ts index 3038e3e6..092c03d2 100644 --- a/src/channel/httpBeaconChannel.ts +++ b/src/channel/httpBeaconChannel.ts @@ -114,7 +114,7 @@ export class HttpBeaconChannel implements DuplexChannel= 500 || status === 429 || status === 408; } } From 0b7aa1a181e69de52d52932997774c5f5d09c93f Mon Sep 17 00:00:00 2001 From: Marcos Passos Date: Fri, 23 Aug 2024 14:08:08 -0600 Subject: [PATCH 15/20] Fix parameter type --- src/channel/channel.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/channel/channel.ts b/src/channel/channel.ts index f8716442..e88a65ac 100644 --- a/src/channel/channel.ts +++ b/src/channel/channel.ts @@ -19,7 +19,7 @@ export class MessageDeliveryError extends Error { return MessageDeliveryError.fromCause(cause, false); } - private static fromCause(cause: any, retryable: boolean): MessageDeliveryError { + private static fromCause(cause: unknown, retryable: boolean): MessageDeliveryError { const error = new MessageDeliveryError(formatMessage(cause), retryable); if (cause instanceof Error) { From 20436954a21ac03cd808412804573670a8b3f702 Mon Sep 17 00:00:00 2001 From: Marcos Passos Date: Fri, 23 Aug 2024 14:09:30 -0600 Subject: [PATCH 16/20] Make constructor public to allow tests --- src/channel/channel.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/channel/channel.ts b/src/channel/channel.ts index e88a65ac..b882ceb4 100644 --- a/src/channel/channel.ts +++ b/src/channel/channel.ts @@ -3,7 +3,7 @@ import {formatMessage} from '../error'; export class MessageDeliveryError extends Error { public readonly retryable: boolean; - private constructor(message: string, retryable: boolean) { + public constructor(message: string, retryable: boolean) { super(message); this.retryable = retryable; From c8c8f4abd75f0a5e6c712cb708a677933498a786 Mon Sep 17 00:00:00 2001 From: Marcos Passos Date: Fri, 23 Aug 2024 14:17:53 -0600 Subject: [PATCH 17/20] Fix test --- src/channel/httpBeaconChannel.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/channel/httpBeaconChannel.ts b/src/channel/httpBeaconChannel.ts index 092c03d2..f2ce6dac 100644 --- a/src/channel/httpBeaconChannel.ts +++ b/src/channel/httpBeaconChannel.ts @@ -85,7 +85,11 @@ export class HttpBeaconChannel implements DuplexChannel { this.logger.error(`Failed to publish beacon: ${formatMessage(error)}`); - return Promise.reject(MessageDeliveryError.retryable(error)); + return Promise.reject( + error instanceof MessageDeliveryError + ? error + : MessageDeliveryError.retryable(error), + ); }); } From 20b964ef73b419d20cfbd82b0e459f7e5f70a0e8 Mon Sep 17 00:00:00 2001 From: Marcos Passos Date: Fri, 23 Aug 2024 15:01:31 -0600 Subject: [PATCH 18/20] Improve error reporting --- src/channel/httpBeaconChannel.ts | 33 ++++++++++++++++-- test/channel/httpBeaconChannel.test.ts | 47 ++++++++++++++++++++++---- 2 files changed, 71 insertions(+), 9 deletions(-) diff --git a/src/channel/httpBeaconChannel.ts b/src/channel/httpBeaconChannel.ts index f2ce6dac..662c3ced 100644 --- a/src/channel/httpBeaconChannel.ts +++ b/src/channel/httpBeaconChannel.ts @@ -73,8 +73,37 @@ export class HttpBeaconChannel implements DuplexChannel { expect(logger.error).toHaveBeenCalledWith('Failed to publish beacon: Internal Server Error'); }); - it.each([ - [401, 'Invalid token'], - [403, 'Unallowed origin'], - [423, 'API usage limit exceeded'], - [402, 'Payment overdue'], - ])('should report a non-retryable error if the response status is %i', async (status, title) => { + type NonRetryableErrorScenario = { + status: number, + title: string, + log: string, + }; + + it.each([ + { + status: 400, + title: 'Invalid token', + log: 'Beacon rejected with non-retryable status: Invalid token', + }, + { + status: 401, + title: 'Unallowed origin', + log: 'The application ID or token is invalid not authorized. ' + + 'For help, see https://croct.help/sdk/js/invalid-credentials', + }, + { + status: 402, + title: 'Payment overdue', + log: 'Beacon rejected with non-retryable status: Payment overdue', + }, + { + status: 403, + title: 'Unallowed origin', + log: 'The origin of the request is not allowed in your application settings. ' + + 'For help, see https://croct.help/sdk/js/invalid-origin', + }, + { + status: 423, + title: 'Quota exceeded', + log: 'The application has exceeded the monthly active users (MAU) quota. ' + + 'For help, see https://croct.help/sdk/js/mau-quota-exceeded', + }, + ])('should report a non-retryable error if the response status is $status', async scenario => { + const {status, title, log} = scenario; + fetchMock.mock(endpointUrl, { status: status, body: JSON.stringify({ @@ -215,7 +247,8 @@ describe('An HTTP beacon channel', () => { expect(listener).not.toHaveBeenCalled(); - expect(logger.error).toHaveBeenCalledWith(`Beacon rejected with non-retryable status: ${title}`); + expect(logger.error).toHaveBeenCalledWith(log); + expect(logger.error).toHaveBeenCalledWith(`Failed to publish beacon: ${title}`); }); it.each([ From d3627922e070a74fdf25c595db14a8305a9e1693 Mon Sep 17 00:00:00 2001 From: Marcos Passos Date: Fri, 23 Aug 2024 15:04:38 -0600 Subject: [PATCH 19/20] Fix typo --- src/channel/httpBeaconChannel.ts | 2 +- test/channel/httpBeaconChannel.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/channel/httpBeaconChannel.ts b/src/channel/httpBeaconChannel.ts index 662c3ced..4772ffea 100644 --- a/src/channel/httpBeaconChannel.ts +++ b/src/channel/httpBeaconChannel.ts @@ -76,7 +76,7 @@ export class HttpBeaconChannel implements DuplexChannel { { status: 401, title: 'Unallowed origin', - log: 'The application ID or token is invalid not authorized. ' + log: 'The application ID or token is not authorized. ' + 'For help, see https://croct.help/sdk/js/invalid-credentials', }, { From 4dd7c1e0c7750d1f1cc55cf116acecdce687f534 Mon Sep 17 00:00:00 2001 From: Marcos Passos Date: Fri, 23 Aug 2024 15:55:45 -0600 Subject: [PATCH 20/20] Update error messages --- src/channel/httpBeaconChannel.ts | 6 +++--- test/channel/httpBeaconChannel.test.ts | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/channel/httpBeaconChannel.ts b/src/channel/httpBeaconChannel.ts index 4772ffea..74bf0302 100644 --- a/src/channel/httpBeaconChannel.ts +++ b/src/channel/httpBeaconChannel.ts @@ -76,7 +76,7 @@ export class HttpBeaconChannel implements DuplexChannel { }, { status: 401, - title: 'Unallowed origin', - log: 'The application ID or token is not authorized. ' + title: 'Unauthorized request', + log: 'The request was not authorized, most likely due to invalid credentials. ' + 'For help, see https://croct.help/sdk/js/invalid-credentials', }, { @@ -194,13 +194,13 @@ describe('An HTTP beacon channel', () => { status: 403, title: 'Unallowed origin', log: 'The origin of the request is not allowed in your application settings. ' - + 'For help, see https://croct.help/sdk/js/invalid-origin', + + 'For help, see https://croct.help/sdk/js/cors', }, { status: 423, title: 'Quota exceeded', log: 'The application has exceeded the monthly active users (MAU) quota. ' - + 'For help, see https://croct.help/sdk/js/mau-quota-exceeded', + + 'For help, see https://croct.help/sdk/js/mau-exceeded', }, ])('should report a non-retryable error if the response status is $status', async scenario => { const {status, title, log} = scenario;