Skip to content

Commit aaeb630

Browse files
authored
feat(core): add ServiceWorker onStateChange telemetry hook (#14867)
* feat(core): add optional onStateChange telemetry hook to ServiceWorker Add an additive, vendor-neutral telemetry hook to the ServiceWorker class. When a consumer supplies register(..., { onStateChange }), the handler is invoked on every service worker statechange event. The existing built-in Pinpoint auto-recording is preserved but now guarded: it only runs when no onStateChange handler is provided, so a supplied handler overrides it and prevents double-recording. The implicit Pinpoint auto-recording is deprecated in favor of onStateChange and will be removed in a future major version. Behavior details: - The handler is captured per register() call, so re-registering with a different handler only affects its own statechange listener. - Consumer handler invocation is wrapped in try/catch; a throwing handler is logged via logger.error and never rejects out of the async listener. Backwards compatible: with no handler, behavior is unchanged. Exports new ServiceWorkerStateChangeHandler type and ServiceWorkerOptions interface from @aws-amplify/core and re-exports them from aws-amplify/utils. * fix(core): await onStateChange handler so async rejections are caught Addresses PR review feedback (review 4653674756): the statechange listener called onStateChange?.(currentState) without awaiting. Since the handler type (state) => void structurally accepts async handlers, a rejected promise from an async handler escaped the synchronous try/catch as an unhandled rejection, contradicting the documented 'never rejects out of the async listener' guarantee. Await the handler inside the try/catch (await undefined resolves immediately for sync/absent handlers) and add a regression test with an async throwing handler. * fix(core): emit current state to onStateChange on register for already-active worker Addresses PR review feedback (review 4653676315 / comment 3543553215): a service worker already in a state when register() resolves (e.g. 'activated' on a repeat visit via the registration.active branch) dispatches no 'statechange' event, so the onStateChange hook never observed the current state. After attaching the statechange listener, emit the current state once to the consumer hook. The emit is intentionally scoped to onStateChange only and does NOT invoke the built-in Pinpoint path, preserving the 'no hook = identical to today' guarantee for existing consumers. Extract a shared _notifyStateChange helper (used by both the listener and the initial emit) and add regression tests for the initial emit and the no-hook no-record-on-register case. * test(core): use real Amplify.configure instead of mocking getConfig
1 parent eed1462 commit aaeb630

6 files changed

Lines changed: 386 additions & 24 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@aws-amplify/core': minor
3+
'aws-amplify': minor
4+
---
5+
6+
feat(core): add optional onStateChange telemetry hook to ServiceWorker.register(); deprecate implicit Pinpoint auto-recording of SW lifecycle events (opt-in, backwards compatible)

packages/aws-amplify/src/utils/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@ export {
1111
Cache,
1212
ConsoleLogger,
1313
ServiceWorker,
14+
ServiceWorkerOptions,
15+
ServiceWorkerStateChangeHandler,
1416
CookieStorage,
1517
defaultStorage,
1618
sessionStorage,

packages/core/__tests__/ServiceWorker.test.ts

Lines changed: 250 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
import { AmplifyError } from '../src/libraryUtils';
22
import { ServiceWorker } from '../src';
33
import { ServiceWorkerErrorCode } from '../src/ServiceWorker/errorHelpers';
4+
import { record } from '../src/providers/pinpoint';
5+
import { Amplify, fetchAuthSession } from '../src/singleton';
6+
import { ConsoleLogger } from '../src/Logger';
7+
8+
jest.mock('../src/providers/pinpoint');
9+
jest.mock('../src/singleton/apis/fetchAuthSession');
410

511
describe('ServiceWorker test', () => {
612
describe('Error conditions', () => {
@@ -168,4 +174,248 @@ describe('ServiceWorker test', () => {
168174
);
169175
});
170176
});
177+
describe('State change telemetry', () => {
178+
const pinpointConfig = {
179+
appId: 'test-app-id',
180+
region: 'us-east-1',
181+
bufferSize: 100,
182+
flushInterval: 1000,
183+
flushSize: 10,
184+
resendLimit: 5,
185+
};
186+
const credentials = {
187+
accessKeyId: 'access-key-id',
188+
secretAccessKey: 'secret-access-key',
189+
};
190+
191+
const registerAndGetStateChangeHandler = async (
192+
onStateChange?: (state: ServiceWorkerState) => void,
193+
): Promise<() => Promise<void>> => {
194+
const mockServiceWorker = {
195+
state: 'activated' as ServiceWorkerState,
196+
addEventListener: jest.fn(),
197+
};
198+
199+
(global as any).navigator.serviceWorker = {
200+
register: () => Promise.resolve({ installing: mockServiceWorker }),
201+
};
202+
203+
const serviceWorker = new ServiceWorker();
204+
await serviceWorker.register(
205+
'/service-worker.js',
206+
'/',
207+
onStateChange ? { onStateChange } : undefined,
208+
);
209+
210+
const [, stateChangeHandler] =
211+
mockServiceWorker.addEventListener.mock.calls.find(
212+
call => call[0] === 'statechange',
213+
) ?? [];
214+
215+
return stateChangeHandler;
216+
};
217+
218+
beforeEach(() => {
219+
jest.clearAllMocks();
220+
Amplify.configure({
221+
Analytics: { Pinpoint: pinpointConfig },
222+
});
223+
(fetchAuthSession as jest.Mock).mockResolvedValue({ credentials });
224+
});
225+
226+
afterAll(() => {
227+
jest.restoreAllMocks();
228+
});
229+
230+
test('records a Pinpoint event when no onStateChange handler is provided', async () => {
231+
const handleStateChange = await registerAndGetStateChangeHandler();
232+
233+
await handleStateChange();
234+
235+
expect(record).toHaveBeenCalledTimes(1);
236+
expect(record).toHaveBeenCalledWith(
237+
expect.objectContaining({
238+
appId: pinpointConfig.appId,
239+
region: pinpointConfig.region,
240+
category: 'Core',
241+
credentials,
242+
event: {
243+
name: 'ServiceWorker',
244+
attributes: { state: 'activated' },
245+
},
246+
}),
247+
);
248+
});
249+
250+
test('invokes onStateChange and suppresses built-in recording when a handler is provided', async () => {
251+
const onStateChange = jest.fn();
252+
const handleStateChange =
253+
await registerAndGetStateChangeHandler(onStateChange);
254+
// Ignore the initial-state emit fired during register(); this test
255+
// isolates the statechange-listener path (covered separately below).
256+
onStateChange.mockClear();
257+
258+
await handleStateChange();
259+
260+
expect(onStateChange).toHaveBeenCalledTimes(1);
261+
expect(onStateChange).toHaveBeenCalledWith('activated');
262+
// The provided handler overrides the built-in path: no double-recording.
263+
expect(fetchAuthSession).not.toHaveBeenCalled();
264+
expect(record).not.toHaveBeenCalled();
265+
});
266+
267+
test('does not record when Pinpoint is not configured and no handler is provided', async () => {
268+
Amplify.configure({});
269+
const handleStateChange = await registerAndGetStateChangeHandler();
270+
271+
await handleStateChange();
272+
273+
expect(record).not.toHaveBeenCalled();
274+
});
275+
276+
test('logs and swallows errors thrown by the onStateChange handler', async () => {
277+
const errorSpy = jest
278+
.spyOn(ConsoleLogger.prototype, 'error')
279+
.mockImplementation(() => undefined);
280+
const thrown = new Error('handler boom');
281+
const onStateChange = jest.fn(() => {
282+
throw thrown;
283+
});
284+
const handleStateChange =
285+
await registerAndGetStateChangeHandler(onStateChange);
286+
// Ignore the initial-state emit fired during register().
287+
onStateChange.mockClear();
288+
errorSpy.mockClear();
289+
290+
// A throwing handler must not reject out of the async listener.
291+
await expect(handleStateChange()).resolves.toBeUndefined();
292+
293+
expect(onStateChange).toHaveBeenCalledWith('activated');
294+
expect(errorSpy).toHaveBeenCalledWith(
295+
'onStateChange handler threw',
296+
thrown,
297+
);
298+
// The built-in path stays suppressed even when the handler throws.
299+
expect(record).not.toHaveBeenCalled();
300+
301+
errorSpy.mockRestore();
302+
});
303+
304+
test('logs and swallows errors thrown by an async onStateChange handler', async () => {
305+
const errorSpy = jest
306+
.spyOn(ConsoleLogger.prototype, 'error')
307+
.mockImplementation(() => undefined);
308+
const thrown = new Error('async handler boom');
309+
const onStateChange = jest.fn(async () => {
310+
throw thrown;
311+
});
312+
const handleStateChange =
313+
await registerAndGetStateChangeHandler(onStateChange);
314+
// Let the initial-state emit's async rejection settle, then ignore it.
315+
await new Promise(resolve => setTimeout(resolve, 0));
316+
onStateChange.mockClear();
317+
errorSpy.mockClear();
318+
319+
// An async handler that rejects must not reject out of the async
320+
// listener (the listener awaits the handler inside the try/catch).
321+
await expect(handleStateChange()).resolves.toBeUndefined();
322+
323+
expect(onStateChange).toHaveBeenCalledWith('activated');
324+
expect(errorSpy).toHaveBeenCalledWith(
325+
'onStateChange handler threw',
326+
thrown,
327+
);
328+
// The built-in path stays suppressed even when the handler rejects.
329+
expect(record).not.toHaveBeenCalled();
330+
331+
errorSpy.mockRestore();
332+
});
333+
334+
test('invokes the handler once per statechange event', async () => {
335+
const onStateChange = jest.fn();
336+
const handleStateChange =
337+
await registerAndGetStateChangeHandler(onStateChange);
338+
// Ignore the initial-state emit fired during register().
339+
onStateChange.mockClear();
340+
341+
await handleStateChange();
342+
await handleStateChange();
343+
await handleStateChange();
344+
345+
expect(onStateChange).toHaveBeenCalledTimes(3);
346+
expect(record).not.toHaveBeenCalled();
347+
});
348+
349+
test('captures the handler per registration so re-register does not re-target a prior listener', async () => {
350+
const handlerA = jest.fn();
351+
const handlerB = jest.fn();
352+
const workerA = {
353+
state: 'activated' as ServiceWorkerState,
354+
addEventListener: jest.fn(),
355+
};
356+
const workerB = {
357+
state: 'activated' as ServiceWorkerState,
358+
addEventListener: jest.fn(),
359+
};
360+
const serviceWorker = new ServiceWorker();
361+
362+
(global as any).navigator.serviceWorker = {
363+
register: () => Promise.resolve({ installing: workerA }),
364+
};
365+
await serviceWorker.register('/service-worker.js', '/', {
366+
onStateChange: handlerA,
367+
});
368+
369+
(global as any).navigator.serviceWorker = {
370+
register: () => Promise.resolve({ installing: workerB }),
371+
};
372+
await serviceWorker.register('/service-worker.js', '/', {
373+
onStateChange: handlerB,
374+
});
375+
376+
// Both registrations fire an initial-state emit; ignore those here so
377+
// this test isolates the per-listener capture behavior.
378+
handlerA.mockClear();
379+
handlerB.mockClear();
380+
381+
const [, listenerA] =
382+
workerA.addEventListener.mock.calls.find(
383+
call => call[0] === 'statechange',
384+
) ?? [];
385+
386+
await listenerA();
387+
388+
// The first listener keeps its own captured handler even after a
389+
// second registration swapped the instance's current handler.
390+
expect(handlerA).toHaveBeenCalledTimes(1);
391+
expect(handlerA).toHaveBeenCalledWith('activated');
392+
expect(handlerB).not.toHaveBeenCalled();
393+
expect(record).not.toHaveBeenCalled();
394+
});
395+
396+
test('emits the current state to the handler on register when the worker is already in a state', async () => {
397+
const onStateChange = jest.fn();
398+
399+
// registerAndGetStateChangeHandler resolves after register() completes;
400+
// the initial-state emit runs synchronously during _setupListeners, so
401+
// the handler has already been notified with the current state without
402+
// any statechange event being dispatched.
403+
await registerAndGetStateChangeHandler(onStateChange);
404+
405+
expect(onStateChange).toHaveBeenCalledTimes(1);
406+
expect(onStateChange).toHaveBeenCalledWith('activated');
407+
// The initial emit must NOT trigger the built-in Pinpoint path; that
408+
// path stays unchanged for consumers that do not pass a handler.
409+
expect(fetchAuthSession).not.toHaveBeenCalled();
410+
expect(record).not.toHaveBeenCalled();
411+
});
412+
413+
test('does not record to Pinpoint on register when no handler is provided (no initial emit)', async () => {
414+
// Without a handler there is no initial emit, and the built-in path is
415+
// only exercised on an actual statechange event, not on register().
416+
await registerAndGetStateChangeHandler();
417+
418+
expect(record).not.toHaveBeenCalled();
419+
});
420+
});
171421
});

0 commit comments

Comments
 (0)