Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion libs/features/org-groups/src/lib/RefreshAllOrgsButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import PQueue from 'p-queue';
import { useState } from 'react';

/** Each health check is a round trip to Salesforce, so fan out enough to stay quick without flooding the user's orgs */
const REFRESH_CONCURRENCY = 4;
const REFRESH_CONCURRENCY = 2;

interface RefreshAllOrgsButtonProps {
className?: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,17 @@ describe('RefreshAllOrgsButton', () => {

expect(button.hasAttribute('disabled')).toBe(true);
expect(button.textContent).toContain('Refreshing 0 of 3');

await act(async () => pendingHealthChecks.forEach((resolve) => resolve()));
// The queue caps in-flight checks, so the remaining orgs stay queued until an earlier one settles
expect(checkOrgHealth).toHaveBeenCalledTimes(2);

// Resolve in waves - each wave lets the queue start the orgs that were still waiting behind it
await act(async () => {
while (pendingHealthChecks.length > 0) {
pendingHealthChecks.splice(0).forEach((resolve) => resolve());
await new Promise((resolve) => setTimeout(resolve, 0));
}
});
expect(checkOrgHealth).toHaveBeenCalledTimes(3);
await waitFor(() => expect(button.hasAttribute('disabled')).toBe(false));
});

Expand Down
11 changes: 10 additions & 1 deletion libs/shared/ui-core/src/app/useInitDataHistory.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import { tracker } from '@jetstream/shared/ui-utils';
import { getErrorMessage } from '@jetstream/shared/utils';
import { dataHistoryCaptureEnabledState, dataHistoryLimitsState } from '@jetstream/ui/app-state';
import { DataHistoryFailureInfo, initDataHistory, setDataHistoryFailureReporter } from '@jetstream/ui/data-history';
import {
DataHistoryFailureInfo,
getDataHistoryErrorDetails,
initDataHistory,
setDataHistoryFailureReporter,
} from '@jetstream/ui/data-history';
import { useSetAtom } from 'jotai';
import { useCallback } from 'react';

Expand All @@ -16,6 +21,10 @@ import { useCallback } from 'react';
*/
function reportDataHistoryFailureToTracker({ operation, error, entryKey, backend }: DataHistoryFailureInfo): void {
tracker.error(`[DATA_HISTORY] ${operation} failed`, error, {
// Spread first so the operation context below always wins on a key collision. Detail the error
// carries (e.g. which worker crashed and where) only reaches the tracker this way — the logger is
// a no-op in production.
...getDataHistoryErrorDetails(error),
entryKey,
backend,
// Included explicitly: FSA and OPFS reject with DOMException, which the tracker cannot unwrap
Expand Down
2 changes: 2 additions & 0 deletions libs/shared/ui-data-history/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,9 @@ export * from './lib/data-history.service';
// Lets the host app point swallowed capture failures at its error tracker (this lib cannot import
// one — see `failure-reporter.ts`)
export {
getDataHistoryErrorDetails,
setDataHistoryFailureReporter,
type DataHistoryErrorDetails,
type DataHistoryFailureInfo,
type DataHistoryFailureOperation,
type DataHistoryFailureReporter,
Expand Down
104 changes: 104 additions & 0 deletions libs/shared/ui-data-history/src/lib/__tests__/opfs-file-store.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { getDataHistoryErrorDetails } from '../failure-reporter';
import { OpfsFileStore } from '../file-store/opfs-file-store';

/**
* Stands in for the real storage worker so the store can be driven through its `onerror` path without
* OPFS (which jsdom does not implement) or a real module load.
*/
class StubWorker {
static instances: StubWorker[] = [];

onmessage: ((event: MessageEvent) => void) | null = null;
onerror: ((event: ErrorEvent) => void) | null = null;
postMessage = vi.fn();
terminate = vi.fn();

constructor() {
StubWorker.instances.push(this);
}
}

const originalWorker = globalThis.Worker;

/**
* Starts a request so there is a pending promise for the crash to reject, and hands back the worker
* that request spawned.
*/
function startPendingRequest() {
const store = new OpfsFileStore('u-0123456789abcdef');
const pending = store.init();
const worker = StubWorker.instances.at(-1);
if (!worker) {
throw new Error('Expected the store to spawn a worker');
}
return { store, pending, worker };
}

describe('OpfsFileStore worker error handling', () => {
beforeEach(() => {
StubWorker.instances = [];
globalThis.Worker = StubWorker as unknown as typeof Worker;
});

afterEach(() => {
globalThis.Worker = originalWorker;
});

it('reports the message and source location of an exception thrown inside the worker', async () => {
const { pending, worker } = startPendingRequest();

worker.onerror?.(
new ErrorEvent('error', {
message: 'Uncaught TypeError: handle.createSyncAccessHandle is not a function',
filename: 'https://getjetstream.app/assets/history-storage.worker-abc123.js',
lineno: 42,
colno: 17,
}),
);

await expect(pending).rejects.toThrow(
'Data history storage worker error: Uncaught TypeError: handle.createSyncAccessHandle is not a function ' +
'(https://getjetstream.app/assets/history-storage.worker-abc123.js:42:17)',
);
});

// A module that fails to load fires a plain `error` Event with no message/filename/lineno/colno,
// which is exactly the crash shape that used to report only "unknown".
it('names the worker module when the error event carries no detail at all', async () => {
const { pending, worker } = startPendingRequest();

worker.onerror?.(new Event('error') as ErrorEvent);

await expect(pending).rejects.toThrow('Data history storage worker error: worker module failed to load (./history-storage.worker.ts)');
});

it('attaches the crash detail to the rejected error so it reaches the error tracker', async () => {
const { pending, worker } = startPendingRequest();

worker.onerror?.(new ErrorEvent('error', { message: 'boom', filename: 'worker.js', lineno: 7, colno: 3 }));

const error = await pending.catch((ex: unknown) => ex);
expect(getDataHistoryErrorDetails(error)).toEqual({
message: 'boom',
filename: 'worker.js',
lineno: 7,
colno: 3,
workerModule: undefined,
});
});

it('terminates the crashed worker and spawns a fresh one for the next request', async () => {
const { store, pending, worker } = startPendingRequest();

worker.onerror?.(new Event('error') as ErrorEvent);
await expect(pending).rejects.toThrow();
expect(worker.terminate).toHaveBeenCalledTimes(1);

const retry = store.init();
expect(StubWorker.instances).toHaveLength(2);

StubWorker.instances[1].onerror?.(new Event('error') as ErrorEvent);
await expect(retry).rejects.toThrow();
});
});
22 changes: 22 additions & 0 deletions libs/shared/ui-data-history/src/lib/failure-reporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,28 @@ export interface DataHistoryFailureInfo {

export type DataHistoryFailureReporter = (info: DataHistoryFailureInfo) => void;

/**
* Diagnostic context attached to an Error so it survives the trip to the error tracker. `logger.warn`
* is a no-op unless the user turns logging on, and the reported Error is the only thing that reaches
* the reporter — so anything a failure needs to be diagnosable has to ride on the Error itself. The
* host app forwards these alongside the operation context (see `useInitDataHistory`).
*/
export type DataHistoryErrorDetails = Record<string, string | number | boolean | undefined>;

const ERROR_DETAILS_KEY = 'dataHistoryErrorDetails';

export function withDataHistoryErrorDetails<TError extends Error>(error: TError, details: DataHistoryErrorDetails): TError {
return Object.assign(error, { [ERROR_DETAILS_KEY]: details });
}

export function getDataHistoryErrorDetails(error: unknown): DataHistoryErrorDetails | undefined {
if (typeof error !== 'object' || error === null) {
return undefined;
}
const details = (error as Record<string, unknown>)[ERROR_DETAILS_KEY];
return typeof details === 'object' && details !== null ? (details as DataHistoryErrorDetails) : undefined;
}

/**
* Conditions where the environment said no rather than Jetstream being broken: the user revoked or
* denied folder access, a picker ran outside a user gesture, or a stream was aborted because the
Expand Down
32 changes: 30 additions & 2 deletions libs/shared/ui-data-history/src/lib/file-store/opfs-file-store.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,15 @@
import { logger } from '@jetstream/shared/client-logger';
import { withDataHistoryErrorDetails } from '../failure-reporter';
import type { HistoryFileStore, HistoryFileStoreCapabilities, HistoryWriteStream } from './file-store.types';
import type { HistoryWorkerRequestBody, HistoryWorkerResponse, HistoryWorkerResultByOp } from './worker-messages';

/**
* Used for error messages only. The bundler emits the worker chunk by pattern-matching the literal
* `new Worker(new URL(...))` call below, so the specifier cannot be shared with it — and the resolved
* href is not obtainable at runtime, since the built worker lives at a hashed path of its own.
*/
const WORKER_MODULE = './history-storage.worker.ts';

/**
* Default Data History file store: OPFS, with all I/O delegated to a dedicated worker
* (`history-storage.worker.ts`) over a small promise-map RPC. The worker is spawned lazily on
Expand Down Expand Up @@ -148,8 +156,28 @@ export class OpfsFileStore implements HistoryFileStore {
this.maybeTerminate();
};
this.worker.onerror = (event) => {
logger.warn('[DATA_HISTORY][OPFS] Storage worker crashed, rejecting pending requests', event.message);
const error = new Error(`Data history storage worker error: ${event.message || 'unknown'}`);
// Neither crash shape hands us the thrown value: an uncaught exception inside a running worker
// arrives as an ErrorEvent whose `error` is always null (the value cannot cross the agent
// boundary), and a worker whose module never loads arrives as a plain `error` Event carrying no
// message, filename, lineno or colno at all. An empty message with no filename is therefore the
// only signal that we are looking at a failed load rather than a crash mid-run.
const moduleLoadFailed = !event.message && !event.filename;
const errorDetails = {
message: event.message || undefined,
filename: event.filename || undefined,
lineno: event.lineno || undefined,
colno: event.colno || undefined,
workerModule: moduleLoadFailed ? WORKER_MODULE : undefined,
};
logger.warn('[DATA_HISTORY][OPFS] Storage worker crashed, rejecting pending requests', errorDetails);
const summary = moduleLoadFailed ? `worker module failed to load (${WORKER_MODULE})` : errorDetails.message || 'unknown';
// lineno/colno are 0 (normalized to undefined above) for load failures, so only append what we actually have.
const location = errorDetails.filename
? ` (${[errorDetails.filename, errorDetails.lineno, errorDetails.colno].filter(Boolean).join(':')})`
: '';
// The detail rides on the Error, not the log line: `logger.warn` is a no-op in production, while
// this Error is what reaches the error tracker through `reportDataHistoryFailure`.
const error = withDataHistoryErrorDetails(new Error(`Data history storage worker error: ${summary}${location}`), errorDetails);
const pending = Array.from(this.pendingRequests.values());
this.pendingRequests.clear();
pending.forEach(({ reject }) => reject(error));
Expand Down
Loading