From 90deed7a3a095de5cfb511796b469406d2ad78df Mon Sep 17 00:00:00 2001 From: ImmutableJeffrey Date: Thu, 9 Apr 2026 11:11:12 +1000 Subject: [PATCH 1/2] refactor(audience-core): replace Transport interface with HttpSend + structured TransportResult MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Transport interface had one production implementation (httpTransport) and was being bypassed anyway: ConsentManager imported httpSend directly instead of going through the injected transport. Tests had to mock at two different levels (jest.fn for queue, jest.mock for consent). Collapse the abstraction: - Delete Transport interface and httpTransport wrapper. - Export type HttpSend = typeof httpSend so tests can mock by injection. - MessageQueue takes a send: HttpSend constructor parameter. - createConsentManager takes a send: HttpSend parameter (new second arg) and stops importing httpSend directly. Same instance flows to both, so the transport layer is finally uniform. - httpSend returns TransportResult { ok, error? } instead of boolean. TransportError carries { status, endpoint, body, cause } for both HTTP failures and network failures. - New errors.ts module hosts TransportError + TransportResult to keep types.ts focused on wire shapes. - Removes the batchSize() helper / payload-shape inspection that lived in transport.ts. The transport no longer cares what it's sending. Behaviour is unchanged: queue still drops messages on result.ok, still retries on failure, still flushes via keepalive on page unload. The HttpSend contract is documented as never-rejects so floating promises in flushUnload and notifyBackend are safe. Test mocks updated to match. The old jest.mock('./transport') in consent.test.ts is gone — fakes are passed by injection now. Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/audience/core/src/consent.test.ts | 40 ++++--- packages/audience/core/src/consent.ts | 10 +- packages/audience/core/src/errors.ts | 33 ++++++ packages/audience/core/src/index.ts | 5 +- packages/audience/core/src/queue.test.ts | 101 ++++++++++-------- packages/audience/core/src/queue.ts | 16 +-- packages/audience/core/src/transport.test.ts | 61 +++++++++-- packages/audience/core/src/transport.ts | 67 +++++++++--- packages/audience/pixel/src/bootstrap.test.ts | 2 +- packages/audience/pixel/src/pixel.test.ts | 13 ++- packages/audience/pixel/src/pixel.ts | 5 +- packages/audience/sdk/src/sdk.ts | 5 +- 12 files changed, 254 insertions(+), 104 deletions(-) create mode 100644 packages/audience/core/src/errors.ts diff --git a/packages/audience/core/src/consent.test.ts b/packages/audience/core/src/consent.test.ts index a52a18cbae..4a235ee5fe 100644 --- a/packages/audience/core/src/consent.test.ts +++ b/packages/audience/core/src/consent.test.ts @@ -1,11 +1,9 @@ import { createConsentManager } from './consent'; -import { httpSend } from './transport'; +import type { HttpSend } from './transport'; -jest.mock('./transport', () => ({ - httpSend: jest.fn().mockResolvedValue(true), -})); - -const mockHttpSend = httpSend as jest.MockedFunction; +function createMockSend(): jest.MockedFunction { + return jest.fn, Parameters>().mockResolvedValue({ ok: true }); +} function createMockQueue() { return { @@ -29,19 +27,22 @@ beforeEach(() => { describe('createConsentManager', () => { it('defaults to none when no initial level provided', () => { const queue = createMockQueue(); - const manager = createConsentManager(queue, 'pk_test', 'anon-1', 'dev', 'pixel'); + const send = createMockSend(); + const manager = createConsentManager(queue, send, 'pk_test', 'anon-1', 'dev', 'pixel'); expect(manager.level).toBe('none'); }); it('uses the initial level when provided', () => { const queue = createMockQueue(); - const manager = createConsentManager(queue, 'pk_test', 'anon-1', 'dev', 'pixel', 'anonymous'); + const send = createMockSend(); + const manager = createConsentManager(queue, send, 'pk_test', 'anon-1', 'dev', 'pixel', 'anonymous'); expect(manager.level).toBe('anonymous'); }); it('upgrades consent without modifying queue', () => { const queue = createMockQueue(); - const manager = createConsentManager(queue, 'pk_test', 'anon-1', 'dev', 'pixel', 'none'); + const send = createMockSend(); + const manager = createConsentManager(queue, send, 'pk_test', 'anon-1', 'dev', 'pixel', 'none'); manager.setLevel('anonymous'); expect(manager.level).toBe('anonymous'); @@ -51,7 +52,8 @@ describe('createConsentManager', () => { it('purges queue on downgrade to none', () => { const queue = createMockQueue(); - const manager = createConsentManager(queue, 'pk_test', 'anon-1', 'dev', 'pixel', 'full'); + const send = createMockSend(); + const manager = createConsentManager(queue, send, 'pk_test', 'anon-1', 'dev', 'pixel', 'full'); manager.setLevel('none'); expect(manager.level).toBe('none'); @@ -64,7 +66,8 @@ describe('createConsentManager', () => { it('strips userId on downgrade from full to anonymous', () => { const queue = createMockQueue(); - const manager = createConsentManager(queue, 'pk_test', 'anon-1', 'dev', 'pixel', 'full'); + const send = createMockSend(); + const manager = createConsentManager(queue, send, 'pk_test', 'anon-1', 'dev', 'pixel', 'full'); manager.setLevel('anonymous'); expect(manager.level).toBe('anonymous'); @@ -78,13 +81,14 @@ describe('createConsentManager', () => { expect(result.anonymousId).toBe('a-1'); }); - it('fires PUT to consent endpoint on level change', () => { + it('fires PUT to consent endpoint on level change via the injected send', () => { const queue = createMockQueue(); - const manager = createConsentManager(queue, 'pk_test', 'anon-1', 'dev', 'pixel', 'none'); + const send = createMockSend(); + const manager = createConsentManager(queue, send, 'pk_test', 'anon-1', 'dev', 'pixel', 'none'); manager.setLevel('anonymous'); - expect(mockHttpSend).toHaveBeenCalledWith( + expect(send).toHaveBeenCalledWith( 'https://api.dev.immutable.com/v1/audience/tracking-consent', 'pk_test', { anonymousId: 'anon-1', status: 'anonymous', source: 'pixel' }, @@ -94,19 +98,21 @@ describe('createConsentManager', () => { it('does nothing when setting the same level', () => { const queue = createMockQueue(); - const manager = createConsentManager(queue, 'pk_test', 'anon-1', 'dev', 'pixel', 'anonymous'); + const send = createMockSend(); + const manager = createConsentManager(queue, send, 'pk_test', 'anon-1', 'dev', 'pixel', 'anonymous'); manager.setLevel('anonymous'); expect(queue.purge).not.toHaveBeenCalled(); expect(queue.transform).not.toHaveBeenCalled(); - expect(mockHttpSend).not.toHaveBeenCalled(); + expect(send).not.toHaveBeenCalled(); }); it('respects DNT by defaulting to none', () => { Object.defineProperty(navigator, 'doNotTrack', { value: '1', configurable: true }); const queue = createMockQueue(); - const manager = createConsentManager(queue, 'pk_test', 'anon-1', 'dev', 'pixel'); + const send = createMockSend(); + const manager = createConsentManager(queue, send, 'pk_test', 'anon-1', 'dev', 'pixel'); expect(manager.level).toBe('none'); Object.defineProperty(navigator, 'doNotTrack', { value: '0', configurable: true }); diff --git a/packages/audience/core/src/consent.ts b/packages/audience/core/src/consent.ts index 02e14095d9..561b2711f8 100644 --- a/packages/audience/core/src/consent.ts +++ b/packages/audience/core/src/consent.ts @@ -2,8 +2,8 @@ import type { ConsentLevel, ConsentUpdatePayload, Message, Environment, } from './types'; import type { MessageQueue } from './queue'; +import type { HttpSend } from './transport'; import { CONSENT_PATH, getBaseUrl } from './config'; -import { httpSend } from './transport'; export interface ConsentManager { level: ConsentLevel; @@ -26,10 +26,13 @@ export function detectDoNotTrack(): boolean { * - If DNT or GPC is detected and no explicit consent is provided, stays `'none'`. * - On downgrade (e.g. full -> anonymous), strips `userId` from queued messages. * - On downgrade to `'none'`, purges the queue entirely. - * - Fires PUT to `/v1/audience/tracking-consent` on every state change. + * - Fires PUT to `/v1/audience/tracking-consent` on every state change via + * the injected `send`. Sharing the same `HttpSend` instance with the queue + * keeps the transport layer uniform — no module-level mocking required. */ export function createConsentManager( queue: MessageQueue, + send: HttpSend, publishableKey: string, anonymousId: string, environment: Environment, @@ -44,7 +47,8 @@ export function createConsentManager( function notifyBackend(level: ConsentLevel): void { const url = `${getBaseUrl(environment)}${CONSENT_PATH}`; const payload: ConsentUpdatePayload = { anonymousId, status: level, source }; - httpSend(url, publishableKey, payload, { method: 'PUT', keepalive: true }); + // Fire-and-forget. HttpSend never rejects, so a floating promise is safe. + send(url, publishableKey, payload, { method: 'PUT', keepalive: true }); } const manager: ConsentManager = { diff --git a/packages/audience/core/src/errors.ts b/packages/audience/core/src/errors.ts new file mode 100644 index 0000000000..3bdca54344 --- /dev/null +++ b/packages/audience/core/src/errors.ts @@ -0,0 +1,33 @@ +/** + * Structured error returned by the transport layer when a send fails. + * + * - `status` is the HTTP status code on a protocol-level error, or `0` + * when the fetch itself rejected (network failure, CORS, DNS). + * - `endpoint` is the full URL that was called. + * - `body` is the parsed response body when content-type was JSON, + * the raw string when not, or undefined when the response had no + * parseable body (e.g. a network failure). + * - `cause` is the original error object (`TypeError`, DOMException, + * etc.) on a network failure; undefined on an HTTP error. + */ +export interface TransportError { + status: number; + endpoint: string; + body?: unknown; + cause?: unknown; +} + +/** + * Return type of every transport send. + * + * `ok: true` means the backend accepted the payload (HTTP 2xx). On + * `ok: false`, `error` is always populated with a structured reason. + * + * Implementations of `HttpSend` MUST NOT reject — failures travel + * through this result type. Callers (notably `MessageQueue.flushUnload`) + * rely on this contract for fire-and-forget paths. + */ +export interface TransportResult { + ok: boolean; + error?: TransportError; +} diff --git a/packages/audience/core/src/index.ts b/packages/audience/core/src/index.ts index 3ab824a4ea..08bb4bf005 100644 --- a/packages/audience/core/src/index.ts +++ b/packages/audience/core/src/index.ts @@ -41,8 +41,9 @@ export { export { generateId, getTimestamp, isBrowser } from './utils'; -export type { Transport, TransportOptions } from './transport'; -export { httpTransport, httpSend } from './transport'; +export type { HttpSend, TransportOptions } from './transport'; +export { httpSend } from './transport'; +export type { TransportError, TransportResult } from './errors'; export { MessageQueue } from './queue'; export { collectContext } from './context'; export { diff --git a/packages/audience/core/src/queue.test.ts b/packages/audience/core/src/queue.test.ts index f5bbd624c3..ed15aebeea 100644 --- a/packages/audience/core/src/queue.test.ts +++ b/packages/audience/core/src/queue.test.ts @@ -1,5 +1,6 @@ import { MessageQueue } from './queue'; -import type { Transport } from './transport'; +import type { HttpSend } from './transport'; +import type { TransportResult } from './errors'; import type { Message } from './types'; import * as storage from './storage'; @@ -15,6 +16,12 @@ function makeMessage(id: string): Message { }; } +const okResult: TransportResult = { ok: true }; +const failResult: TransportResult = { + ok: false, + error: { status: 500, endpoint: 'https://example.com', body: null }, +}; + interface QueueOpts { flushIntervalMs?: number; flushSize?: number; @@ -23,11 +30,11 @@ interface QueueOpts { } function createQueue( - transport: Transport, + send: HttpSend, opts: QueueOpts = {}, ) { return new MessageQueue( - transport, + send, 'https://api.immutable.com/v1/audience/messages', 'pk_imx_test', opts.flushIntervalMs ?? 5_000, @@ -47,8 +54,8 @@ afterEach(() => { describe('MessageQueue', () => { it('enqueues messages and flushes them', async () => { - const send = jest.fn().mockResolvedValue(true); - const queue = createQueue({ send }); + const send = jest.fn, Parameters>().mockResolvedValue(okResult); + const queue = createQueue(send); queue.enqueue(makeMessage('1')); queue.enqueue(makeMessage('2')); @@ -56,13 +63,13 @@ describe('MessageQueue', () => { await queue.flush(); expect(send).toHaveBeenCalledTimes(1); - expect(send.mock.calls[0][2].messages).toHaveLength(2); + expect((send.mock.calls[0][2] as { messages: Message[] }).messages).toHaveLength(2); expect(queue.length).toBe(0); }); it('retains messages on failed flush', async () => { - const send = jest.fn().mockResolvedValue(false); - const queue = createQueue({ send }); + const send = jest.fn, Parameters>().mockResolvedValue(failResult); + const queue = createQueue(send); queue.enqueue(makeMessage('1')); await queue.flush(); @@ -71,8 +78,8 @@ describe('MessageQueue', () => { }); it('flushes automatically when batch size is reached', async () => { - const send = jest.fn().mockResolvedValue(true); - const queue = createQueue({ send }, { flushSize: 2 }); + const send = jest.fn, Parameters>().mockResolvedValue(okResult); + const queue = createQueue(send, { flushSize: 2 }); queue.enqueue(makeMessage('1')); expect(send).not.toHaveBeenCalled(); @@ -84,8 +91,8 @@ describe('MessageQueue', () => { }); it('flushes on timer interval', async () => { - const send = jest.fn().mockResolvedValue(true); - const queue = createQueue({ send }, { flushIntervalMs: 1_000 }); + const send = jest.fn, Parameters>().mockResolvedValue(okResult); + const queue = createQueue(send, { flushIntervalMs: 1_000 }); queue.start(); queue.enqueue(makeMessage('1')); @@ -99,8 +106,8 @@ describe('MessageQueue', () => { }); it('persists messages to localStorage', () => { - const send = jest.fn().mockResolvedValue(true); - const queue = createQueue({ send }); + const send = jest.fn, Parameters>().mockResolvedValue(okResult); + const queue = createQueue(send); queue.enqueue(makeMessage('1')); @@ -112,8 +119,8 @@ describe('MessageQueue', () => { it('restores messages from localStorage on construction', () => { storage.setItem('queue', [makeMessage('restored')]); - const send = jest.fn().mockResolvedValue(true); - const queue = createQueue({ send }); + const send = jest.fn, Parameters>().mockResolvedValue(okResult); + const queue = createQueue(send); expect(queue.length).toBe(1); }); @@ -121,8 +128,8 @@ describe('MessageQueue', () => { it('filters stale messages on restore', () => { storage.setItem('queue', [makeMessage('stale'), makeMessage('fresh')]); - const send = jest.fn().mockResolvedValue(true); - const queue = createQueue({ send }, { + const send = jest.fn, Parameters>().mockResolvedValue(okResult); + const queue = createQueue(send, { staleFilter: (m) => m.messageId === 'fresh', }); @@ -131,12 +138,12 @@ describe('MessageQueue', () => { it('does not flush concurrently', async () => { let resolveFirst: () => void; - const firstCall = new Promise((r) => { resolveFirst = () => r(true); }); - const send = jest.fn() + const firstCall = new Promise((r) => { resolveFirst = () => r(okResult); }); + const send = jest.fn, Parameters>() .mockReturnValueOnce(firstCall) - .mockResolvedValue(true); + .mockResolvedValue(okResult); - const queue = createQueue({ send }); + const queue = createQueue(send); queue.enqueue(makeMessage('1')); const flush1 = queue.flush(); @@ -150,8 +157,8 @@ describe('MessageQueue', () => { }); it('clears all messages and storage', () => { - const send = jest.fn().mockResolvedValue(true); - const queue = createQueue({ send }); + const send = jest.fn, Parameters>().mockResolvedValue(okResult); + const queue = createQueue(send); queue.enqueue(makeMessage('1')); queue.clear(); @@ -162,12 +169,12 @@ describe('MessageQueue', () => { it('handles messages enqueued during flush', async () => { let queue: ReturnType; - const send = jest.fn().mockImplementation(async () => { + const send = jest.fn, Parameters>().mockImplementation(async () => { queue.enqueue(makeMessage('late')); - return true; + return okResult; }); - queue = createQueue({ send }); + queue = createQueue(send); queue.enqueue(makeMessage('1')); await queue.flush(); @@ -177,8 +184,8 @@ describe('MessageQueue', () => { it('calls onFlush callback', async () => { const onFlush = jest.fn(); - const send = jest.fn().mockResolvedValue(true); - const queue = createQueue({ send }, { onFlush }); + const send = jest.fn, Parameters>().mockResolvedValue(okResult); + const queue = createQueue(send, { onFlush }); queue.enqueue(makeMessage('1')); await queue.flush(); @@ -187,8 +194,8 @@ describe('MessageQueue', () => { }); it('purges messages matching a predicate', () => { - const send = jest.fn().mockResolvedValue(true); - const queue = createQueue({ send }); + const send = jest.fn, Parameters>().mockResolvedValue(okResult); + const queue = createQueue(send); queue.enqueue(makeMessage('1')); queue.enqueue({ ...makeMessage('2'), type: 'identify' } as any); @@ -199,8 +206,8 @@ describe('MessageQueue', () => { }); it('transforms messages in place', async () => { - const send = jest.fn().mockResolvedValue(true); - const queue = createQueue({ send }); + const send = jest.fn, Parameters>().mockResolvedValue(okResult); + const queue = createQueue(send); queue.enqueue({ ...makeMessage('1'), userId: 'should-strip' } as any); @@ -211,15 +218,15 @@ describe('MessageQueue', () => { }); await queue.flush(); - const msg = send.mock.calls[0][2].messages[0]; + const msg = (send.mock.calls[0][2] as { messages: Message[] }).messages[0]; expect((msg as any).userId).toBeUndefined(); }); }); describe('page-unload flush (keepalive)', () => { it('flushes via keepalive fetch on visibilitychange to hidden', () => { - const send = jest.fn().mockResolvedValue(true); - const queue = createQueue({ send }); + const send = jest.fn, Parameters>().mockResolvedValue(okResult); + const queue = createQueue(send); queue.start(); queue.enqueue(makeMessage('1')); @@ -248,8 +255,8 @@ describe('page-unload flush (keepalive)', () => { }); it('flushes via keepalive fetch on pagehide', () => { - const send = jest.fn().mockResolvedValue(true); - const queue = createQueue({ send }); + const send = jest.fn, Parameters>().mockResolvedValue(okResult); + const queue = createQueue(send); queue.start(); queue.enqueue(makeMessage('1')); @@ -267,8 +274,8 @@ describe('page-unload flush (keepalive)', () => { }); it('does not fire unload flush when queue is empty', () => { - const send = jest.fn().mockResolvedValue(true); - const queue = createQueue({ send }); + const send = jest.fn, Parameters>().mockResolvedValue(okResult); + const queue = createQueue(send); queue.start(); window.dispatchEvent(new Event('pagehide')); @@ -279,8 +286,8 @@ describe('page-unload flush (keepalive)', () => { }); it('removes listeners on stop', () => { - const send = jest.fn().mockResolvedValue(true); - const queue = createQueue({ send }); + const send = jest.fn, Parameters>().mockResolvedValue(okResult); + const queue = createQueue(send); queue.start(); queue.stop(); @@ -291,8 +298,8 @@ describe('page-unload flush (keepalive)', () => { }); it('destroy stops the queue and flushes remaining messages', () => { - const send = jest.fn().mockResolvedValue(true); - const queue = createQueue({ send }); + const send = jest.fn, Parameters>().mockResolvedValue(okResult); + const queue = createQueue(send); queue.start(); queue.enqueue(makeMessage('1')); @@ -315,10 +322,10 @@ describe('page-unload flush (keepalive)', () => { it('skips unload flush if an async flush is already in flight', async () => { let resolveFlush: () => void; - const flushPromise = new Promise((r) => { resolveFlush = () => r(true); }); - const send = jest.fn().mockReturnValueOnce(flushPromise); + const flushPromise = new Promise((r) => { resolveFlush = () => r(okResult); }); + const send = jest.fn, Parameters>().mockReturnValueOnce(flushPromise); - const queue = createQueue({ send }); + const queue = createQueue(send); queue.start(); queue.enqueue(makeMessage('1')); diff --git a/packages/audience/core/src/queue.ts b/packages/audience/core/src/queue.ts index c8c0508fe2..e2e97602bd 100644 --- a/packages/audience/core/src/queue.ts +++ b/packages/audience/core/src/queue.ts @@ -1,5 +1,5 @@ import type { Message, BatchPayload } from './types'; -import type { Transport } from './transport'; +import type { HttpSend } from './transport'; import * as storage from './storage'; import { isBrowser } from './utils'; @@ -53,7 +53,7 @@ export class MessageQueue { private readonly storagePrefix?: string; constructor( - private readonly transport: Transport, + private readonly send: HttpSend, private readonly endpointUrl: string, private readonly publishableKey: string, private readonly flushIntervalMs: number, @@ -113,12 +113,12 @@ export class MessageQueue { const batch = this.messages.slice(0, MAX_BATCH_SIZE); const payload: BatchPayload = { messages: batch }; - const ok = await this.transport.send(this.endpointUrl, this.publishableKey, payload); - if (ok) { + const result = await this.send(this.endpointUrl, this.publishableKey, payload); + if (result.ok) { this.messages = this.messages.slice(batch.length); this.persist(); } - this.onFlush?.(ok, batch.length); + this.onFlush?.(result.ok, batch.length); } finally { this.flushing = false; } @@ -138,7 +138,11 @@ export class MessageQueue { const batch = this.messages.slice(0, MAX_BATCH_SIZE); const payload: BatchPayload = { messages: batch }; - this.transport.send(this.endpointUrl, this.publishableKey, payload, { keepalive: true }); + // Fire-and-forget — `keepalive: true` lets the request survive page + // navigation. We optimistically drop the batch because the page is + // going away and can't retry. The HttpSend contract guarantees this + // promise never rejects, so the floating call is safe. + this.send(this.endpointUrl, this.publishableKey, payload, { keepalive: true }); this.messages = this.messages.slice(batch.length); this.persist(); } diff --git a/packages/audience/core/src/transport.test.ts b/packages/audience/core/src/transport.test.ts index a1717f947f..c03315ce14 100644 --- a/packages/audience/core/src/transport.test.ts +++ b/packages/audience/core/src/transport.test.ts @@ -65,18 +65,63 @@ describe('httpSend', () => { })); }); - it('returns true on success', async () => { + it('returns ok on 2xx response', async () => { global.fetch = jest.fn().mockResolvedValue({ ok: true }); - expect(await httpSend('https://example.com', 'pk', payload)).toBe(true); + const result = await httpSend('https://example.com', 'pk', payload); + expect(result.ok).toBe(true); + expect(result.error).toBeUndefined(); }); - it('returns false on HTTP error', async () => { - global.fetch = jest.fn().mockResolvedValue({ ok: false, status: 500 }); - expect(await httpSend('https://example.com', 'pk', payload)).toBe(false); + it('returns structured error on HTTP failure with parsed JSON body', async () => { + global.fetch = jest.fn().mockResolvedValue({ + ok: false, + status: 422, + headers: { get: () => 'application/json' }, + json: async () => ({ code: 'INVALID_PAYLOAD' }), + }); + + const result = await httpSend('https://example.com', 'pk', payload); + + expect(result.ok).toBe(false); + expect(result.error).toEqual({ + status: 422, + endpoint: 'https://example.com', + body: { code: 'INVALID_PAYLOAD' }, + }); }); - it('returns false on network error', async () => { - global.fetch = jest.fn().mockRejectedValue(new TypeError('Failed to fetch')); - expect(await httpSend('https://example.com', 'pk', payload)).toBe(false); + it('returns structured error on HTTP failure with text body', async () => { + global.fetch = jest.fn().mockResolvedValue({ + ok: false, + status: 500, + headers: { get: () => 'text/plain' }, + text: async () => 'Internal Server Error', + }); + + const result = await httpSend('https://example.com', 'pk', payload); + + expect(result.ok).toBe(false); + expect(result.error?.status).toBe(500); + expect(result.error?.body).toBe('Internal Server Error'); + }); + + it('returns structured error with status 0 and cause on network failure', async () => { + const networkError = new TypeError('Failed to fetch'); + global.fetch = jest.fn().mockRejectedValue(networkError); + + const result = await httpSend('https://example.com', 'pk', payload); + + expect(result.ok).toBe(false); + expect(result.error?.status).toBe(0); + expect(result.error?.endpoint).toBe('https://example.com'); + expect(result.error?.cause).toBe(networkError); + }); + + it('never rejects — even when fetch throws synchronously', async () => { + global.fetch = jest.fn().mockImplementation(() => { + throw new Error('synchronous boom'); + }); + + await expect(httpSend('https://example.com', 'pk', payload)).resolves.toBeDefined(); }); }); diff --git a/packages/audience/core/src/transport.ts b/packages/audience/core/src/transport.ts index 6be431dafd..6fbeb835f6 100644 --- a/packages/audience/core/src/transport.ts +++ b/packages/audience/core/src/transport.ts @@ -1,21 +1,48 @@ import { track, trackError } from '@imtbl/metrics'; import type { BatchPayload, ConsentUpdatePayload } from './types'; +import type { TransportError, TransportResult } from './errors'; export interface TransportOptions { method?: string; keepalive?: boolean; } -export interface Transport { - send(url: string, publishableKey: string, payload: BatchPayload, options?: TransportOptions): Promise; -} - -export async function httpSend( +/** + * Function type for sending payloads to the audience backend. + * + * The single production implementation is {@link httpSend}. Inject this + * type into `MessageQueue` and `createConsentManager` so tests can + * substitute a fake by passing `jest.fn()` directly. + * + * Implementations MUST NOT reject — failures are returned via + * {@link TransportResult}. Callers rely on this contract for + * fire-and-forget code paths (page-unload flush). + */ +export type HttpSend = ( url: string, publishableKey: string, payload: BatchPayload | ConsentUpdatePayload, options?: TransportOptions, -): Promise { +) => Promise; + +async function parseBody(response: Response): Promise { + const contentType = response.headers?.get?.('content-type') ?? ''; + try { + if (contentType.includes('application/json')) { + return await response.json(); + } + return await response.text(); + } catch { + return undefined; + } +} + +export const httpSend: HttpSend = async ( + url, + publishableKey, + payload, + options, +) => { try { const response = await fetch(url, { method: options?.method ?? 'POST', @@ -28,14 +55,28 @@ export async function httpSend( }); if (!response.ok) { + const body = await parseBody(response); track('audience', 'transport_send_failed', { status: response.status }); + const error: TransportError = { + status: response.status, + endpoint: url, + body, + }; + return { ok: false, error }; } - return response.ok; - } catch (error) { - trackError('audience', 'transport_send', error instanceof Error ? error : new Error(String(error))); - return false; + return { ok: true }; + } catch (err) { + const causeError = err instanceof Error ? err : new Error(String(err)); + trackError('audience', 'transport_send', causeError); + return { + ok: false, + error: { + status: 0, + endpoint: url, + body: undefined, + cause: err, + }, + }; } -} - -export const httpTransport: Transport = { send: httpSend }; +}; diff --git a/packages/audience/pixel/src/bootstrap.test.ts b/packages/audience/pixel/src/bootstrap.test.ts index f811564609..1cd7598982 100644 --- a/packages/audience/pixel/src/bootstrap.test.ts +++ b/packages/audience/pixel/src/bootstrap.test.ts @@ -21,7 +21,7 @@ jest.mock('./pixel', () => ({ jest.mock('@imtbl/audience-core', () => ({ MessageQueue: jest.fn(), - httpTransport: {}, + httpSend: jest.fn(), getBaseUrl: jest.fn(), INGEST_PATH: '', FLUSH_INTERVAL_MS: 5000, diff --git a/packages/audience/pixel/src/pixel.test.ts b/packages/audience/pixel/src/pixel.test.ts index 4bb9785357..3b7ee88463 100644 --- a/packages/audience/pixel/src/pixel.test.ts +++ b/packages/audience/pixel/src/pixel.test.ts @@ -28,8 +28,7 @@ jest.mock('@imtbl/audience-core', () => ({ clear: jest.fn(), get length() { return 0; }, })), - httpTransport: { send: jest.fn().mockResolvedValue(true) }, - httpSend: jest.fn().mockResolvedValue(true), + httpSend: jest.fn().mockResolvedValue({ ok: true }), getBaseUrl: jest.fn().mockReturnValue('https://api.dev.immutable.com'), INGEST_PATH: '/v1/audience/messages', CONSENT_PATH: '/v1/audience/tracking-consent', @@ -52,7 +51,15 @@ jest.mock('@imtbl/audience-core', () => ({ }), getOrCreateSession: (...args: unknown[]) => mockGetOrCreateSession(...args), createConsentManager: jest.fn().mockImplementation( - (_queue: unknown, _key: unknown, _anonId: unknown, _env: unknown, _source: unknown, level?: string) => { + ( + _queue: unknown, + _send: unknown, + _key: unknown, + _anonId: unknown, + _env: unknown, + _source: unknown, + level?: string, + ) => { let current = level ?? 'none'; return { get level() { return current; }, diff --git a/packages/audience/pixel/src/pixel.ts b/packages/audience/pixel/src/pixel.ts index 99de797555..96c006dc0a 100644 --- a/packages/audience/pixel/src/pixel.ts +++ b/packages/audience/pixel/src/pixel.ts @@ -9,7 +9,7 @@ import type { } from '@imtbl/audience-core'; import { MessageQueue, - httpTransport, + httpSend, getBaseUrl, INGEST_PATH, FLUSH_INTERVAL_MS, @@ -84,7 +84,7 @@ export class Pixel { const endpointUrl = `${getBaseUrl(environment)}${INGEST_PATH}`; this.queue = new MessageQueue( - httpTransport, + httpSend, endpointUrl, key, FLUSH_INTERVAL_MS, @@ -96,6 +96,7 @@ export class Pixel { this.consent = createConsentManager( this.queue, + httpSend, key, this.anonymousId, environment, diff --git a/packages/audience/sdk/src/sdk.ts b/packages/audience/sdk/src/sdk.ts index ead8e3b9e7..fe77bc1bf7 100644 --- a/packages/audience/sdk/src/sdk.ts +++ b/packages/audience/sdk/src/sdk.ts @@ -13,7 +13,7 @@ import { COOKIE_NAME, SESSION_COOKIE, MessageQueue, - httpTransport, + httpSend, getBaseUrl, getOrCreateAnonymousId, getCookie, @@ -91,7 +91,7 @@ export class Audience { const endpointUrl = `${getBaseUrl(environment)}${INGEST_PATH}`; this.queue = new MessageQueue( - httpTransport, + httpSend, endpointUrl, publishableKey, flushInterval, @@ -105,6 +105,7 @@ export class Audience { this.consent = createConsentManager( this.queue, + httpSend, publishableKey, this.anonymousId, environment, From 518db0e1beb44e44b596183ca97ac53a022f982d Mon Sep 17 00:00:00 2001 From: ImmutableJeffrey Date: Thu, 9 Apr 2026 13:28:00 +1000 Subject: [PATCH 2/2] refactor(audience-core): make TransportError an Error subclass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review feedback (bkbooth): stack traces were lost because TransportError was a plain interface. Instances now carry a native stack and integrate with devtools / Sentry / @imtbl/metrics without custom wrapping. - Convert TransportError from interface to class extends Error. - Use ES2022 Error.cause via super(msg, { cause }) — the existing .cause field comes for free, source-compatible with consumers. - status / endpoint / body become readonly class fields; name and message set so default stringification is useful. - Two construction sites in transport.ts switch to new TransportError. - Collapse the causeError workaround in the catch branch: the TransportError IS an Error, so trackError consumes it directly and the original error is preserved via cause chaining. Tests: transport.test.ts uses toMatchObject + toBeInstanceOf and adds a lock-in test for the Error contract (instanceof Error, name, stack). queue.test.ts fail fixture constructs via new TransportError({...}). No behaviour change visible to studios — field access on error.status / endpoint / body / cause is identical (cause is native Error.cause, accessed the same way). Co-Authored-By: Claude Opus 4.6 (1M context) --- packages/audience/core/src/errors.ts | 37 ++++++++++++++++---- packages/audience/core/src/queue.test.ts | 8 +++-- packages/audience/core/src/transport.test.ts | 19 +++++++++- packages/audience/core/src/transport.ts | 32 ++++++++--------- 4 files changed, 69 insertions(+), 27 deletions(-) diff --git a/packages/audience/core/src/errors.ts b/packages/audience/core/src/errors.ts index 3bdca54344..11ad1d9d47 100644 --- a/packages/audience/core/src/errors.ts +++ b/packages/audience/core/src/errors.ts @@ -1,20 +1,43 @@ /** * Structured error returned by the transport layer when a send fails. * + * Extends native {@link Error} so stack traces are captured at the + * construction site and instances flow through standard error-reporting + * tools (browser devtools, Sentry, `@imtbl/metrics`) without custom + * handling. + * * - `status` is the HTTP status code on a protocol-level error, or `0` * when the fetch itself rejected (network failure, CORS, DNS). * - `endpoint` is the full URL that was called. * - `body` is the parsed response body when content-type was JSON, * the raw string when not, or undefined when the response had no * parseable body (e.g. a network failure). - * - `cause` is the original error object (`TypeError`, DOMException, - * etc.) on a network failure; undefined on an HTTP error. + * - `cause` (inherited from {@link Error}, ES2022) is the original error + * object (`TypeError`, `DOMException`, etc.) on a network failure; + * undefined on an HTTP error. */ -export interface TransportError { - status: number; - endpoint: string; - body?: unknown; - cause?: unknown; +export class TransportError extends Error { + readonly status: number; + + readonly endpoint: string; + + readonly body?: unknown; + + constructor(init: { + status: number; + endpoint: string; + body?: unknown; + cause?: unknown; + }) { + super( + `audience transport failed: ${init.status || 'network error'} ${init.endpoint}`, + init.cause !== undefined ? { cause: init.cause } : undefined, + ); + this.name = 'TransportError'; + this.status = init.status; + this.endpoint = init.endpoint; + this.body = init.body; + } } /** diff --git a/packages/audience/core/src/queue.test.ts b/packages/audience/core/src/queue.test.ts index ed15aebeea..d1811a2f61 100644 --- a/packages/audience/core/src/queue.test.ts +++ b/packages/audience/core/src/queue.test.ts @@ -1,6 +1,6 @@ import { MessageQueue } from './queue'; import type { HttpSend } from './transport'; -import type { TransportResult } from './errors'; +import { TransportError, type TransportResult } from './errors'; import type { Message } from './types'; import * as storage from './storage'; @@ -19,7 +19,11 @@ function makeMessage(id: string): Message { const okResult: TransportResult = { ok: true }; const failResult: TransportResult = { ok: false, - error: { status: 500, endpoint: 'https://example.com', body: null }, + error: new TransportError({ + status: 500, + endpoint: 'https://example.com', + body: null, + }), }; interface QueueOpts { diff --git a/packages/audience/core/src/transport.test.ts b/packages/audience/core/src/transport.test.ts index c03315ce14..a446a69cbd 100644 --- a/packages/audience/core/src/transport.test.ts +++ b/packages/audience/core/src/transport.test.ts @@ -1,4 +1,5 @@ import { httpSend } from './transport'; +import { TransportError } from './errors'; import type { BatchPayload } from './types'; const payload: BatchPayload = { @@ -83,13 +84,29 @@ describe('httpSend', () => { const result = await httpSend('https://example.com', 'pk', payload); expect(result.ok).toBe(false); - expect(result.error).toEqual({ + expect(result.error).toBeInstanceOf(TransportError); + expect(result.error).toMatchObject({ status: 422, endpoint: 'https://example.com', body: { code: 'INVALID_PAYLOAD' }, }); }); + it('TransportError is a real Error with a stack trace', async () => { + global.fetch = jest.fn().mockResolvedValue({ + ok: false, + status: 500, + headers: { get: () => 'text/plain' }, + text: async () => 'boom', + }); + + const result = await httpSend('https://example.com', 'pk', payload); + + expect(result.error).toBeInstanceOf(Error); + expect(result.error?.name).toBe('TransportError'); + expect(typeof result.error?.stack).toBe('string'); + }); + it('returns structured error on HTTP failure with text body', async () => { global.fetch = jest.fn().mockResolvedValue({ ok: false, diff --git a/packages/audience/core/src/transport.ts b/packages/audience/core/src/transport.ts index 6fbeb835f6..6b3ddea7a1 100644 --- a/packages/audience/core/src/transport.ts +++ b/packages/audience/core/src/transport.ts @@ -1,6 +1,6 @@ import { track, trackError } from '@imtbl/metrics'; import type { BatchPayload, ConsentUpdatePayload } from './types'; -import type { TransportError, TransportResult } from './errors'; +import { TransportError, type TransportResult } from './errors'; export interface TransportOptions { method?: string; @@ -57,26 +57,24 @@ export const httpSend: HttpSend = async ( if (!response.ok) { const body = await parseBody(response); track('audience', 'transport_send_failed', { status: response.status }); - const error: TransportError = { - status: response.status, - endpoint: url, - body, + return { + ok: false, + error: new TransportError({ + status: response.status, + endpoint: url, + body, + }), }; - return { ok: false, error }; } return { ok: true }; } catch (err) { - const causeError = err instanceof Error ? err : new Error(String(err)); - trackError('audience', 'transport_send', causeError); - return { - ok: false, - error: { - status: 0, - endpoint: url, - body: undefined, - cause: err, - }, - }; + const error = new TransportError({ + status: 0, + endpoint: url, + cause: err, + }); + trackError('audience', 'transport_send', error); + return { ok: false, error }; } };