diff --git a/.env.example b/.env.example index 4d4de8e2b..b0b7f1fc2 100644 --- a/.env.example +++ b/.env.example @@ -100,6 +100,9 @@ EXAMPLE_USER_PASSWORD='EXAMPLE_123!' DEFERRED_RESPONSE_ENABLED='false' # DEFERRED_RESPONSE_THRESHOLD_MS='75000' # DEFERRED_RESPONSE_KEEPALIVE_MS='25000' +# Backstop for a silent upstream — stops keepalives and returns an error body. +# Measured from when the request arrived, so it can be compared directly against upstream timeouts. +# DEFERRED_RESPONSE_MAX_DURATION_MS='600000' # Set to true to enable test endpoints (e.g. /api/test/deferred-response) # ENABLE_TEST_ENDPOINTS='false' diff --git a/apps/api/src/app/types/route.types.ts b/apps/api/src/app/types/route.types.ts index 7ce1b8126..802168218 100644 --- a/apps/api/src/app/types/route.types.ts +++ b/apps/api/src/app/types/route.types.ts @@ -35,8 +35,14 @@ export type Request< */ export interface DeferredResponseState { active: boolean; + /** + * The max duration backstop gave up on the upstream and ended the response. The controller may still + * finish afterwards and try to respond — that is expected here, not an unhandled response. + */ + abandoned: boolean; timer: NodeJS.Timeout | null; keepaliveInterval: NodeJS.Timeout | null; + maxDurationTimer: NodeJS.Timeout | null; startTime: number; keepaliveCount: number; } diff --git a/apps/api/src/app/utils/__tests__/deferred-response.middleware.spec.ts b/apps/api/src/app/utils/__tests__/deferred-response.middleware.spec.ts index 316f43832..f8aae8a78 100644 --- a/apps/api/src/app/utils/__tests__/deferred-response.middleware.spec.ts +++ b/apps/api/src/app/utils/__tests__/deferred-response.middleware.spec.ts @@ -12,6 +12,7 @@ vi.mock('@jetstream/api-config', () => ({ DEFERRED_RESPONSE_ENABLED: true, DEFERRED_RESPONSE_THRESHOLD_MS: 75_000, DEFERRED_RESPONSE_KEEPALIVE_MS: 25_000, + DEFERRED_RESPONSE_MAX_DURATION_MS: 600_000, }, getLogger: () => loggerHolder.current, })); @@ -307,8 +308,10 @@ describe('writeDeferredResponse', () => { const res = createMockRes(); const deferred: DeferredResponseState = { active: true, + abandoned: false, timer: null, keepaliveInterval: null, + maxDurationTimer: null, startTime: Date.now() - 50_000, keepaliveCount: 2, }; @@ -327,8 +330,10 @@ describe('writeDeferredResponse', () => { const res = createMockRes(); const deferred: DeferredResponseState = { active: true, + abandoned: false, timer: null, keepaliveInterval: null, + maxDurationTimer: null, startTime: Date.now() - 60_000, keepaliveCount: 3, }; @@ -350,8 +355,10 @@ describe('writeDeferredResponse', () => { }); const deferred: DeferredResponseState = { active: true, + abandoned: false, timer: null, keepaliveInterval: null, + maxDurationTimer: null, startTime: Date.now() - 50_000, keepaliveCount: 2, }; @@ -372,8 +379,10 @@ describe('writeDeferredResponse', () => { const res = createMockRes(); const deferred: DeferredResponseState = { active: true, + abandoned: false, timer: null, keepaliveInterval: null, + maxDurationTimer: null, startTime: Date.now() - 50_000, keepaliveCount: 2, }; @@ -405,8 +414,10 @@ describe('writeDeferredResponse', () => { }); const deferred: DeferredResponseState = { active: true, + abandoned: false, timer: null, keepaliveInterval: null, + maxDurationTimer: null, startTime: Date.now() - 50_000, keepaliveCount: 2, }; @@ -430,8 +441,10 @@ describe('writeDeferredResponse', () => { }); const deferred: DeferredResponseState = { active: true, + abandoned: false, timer: null, keepaliveInterval: null, + maxDurationTimer: null, startTime: Date.now() - 50_000, keepaliveCount: 2, }; @@ -449,10 +462,13 @@ describe('writeDeferredResponse', () => { const res = createMockRes(); const timer = setTimeout(() => {}, 10_000); const interval = setInterval(() => {}, 5_000); + const maxDurationTimer = setTimeout(() => {}, 600_000); const deferred: DeferredResponseState = { active: true, + abandoned: false, timer, keepaliveInterval: interval, + maxDurationTimer, startTime: Date.now() - 50_000, keepaliveCount: 2, }; @@ -462,6 +478,7 @@ describe('writeDeferredResponse', () => { expect(deferred.timer).toBeNull(); expect(deferred.keepaliveInterval).toBeNull(); + expect(deferred.maxDurationTimer).toBeNull(); vi.useRealTimers(); }); @@ -469,8 +486,10 @@ describe('writeDeferredResponse', () => { const res = createMockRes(); const deferred: DeferredResponseState = { active: true, + abandoned: false, timer: null, keepaliveInterval: null, + maxDurationTimer: null, startTime: Date.now() - 50_000, keepaliveCount: 2, }; @@ -487,8 +506,10 @@ describe('writeDeferredResponse', () => { const res = createMockRes(); const deferred: DeferredResponseState = { active: true, + abandoned: false, timer: null, keepaliveInterval: null, + maxDurationTimer: null, startTime: Date.now() - 50_000, keepaliveCount: 2, }; @@ -505,8 +526,10 @@ describe('writeDeferredResponse', () => { const res = createMockRes(); const deferred: DeferredResponseState = { active: true, + abandoned: false, timer: null, keepaliveInterval: null, + maxDurationTimer: null, startTime: Date.now() - 50_000, keepaliveCount: 2, }; @@ -521,6 +544,99 @@ describe('writeDeferredResponse', () => { }); }); +describe('deferred response max duration backstop', () => { + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + vi.restoreAllMocks(); + }); + + it('should abandon the response with an error body once max duration elapses', () => { + const req = createMockReq(); + const res = createMockRes(); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + deferredResponseMiddleware(req as any, res as any, vi.fn()); + vi.advanceTimersByTime(75_000); + expect((res.locals._deferred as DeferredResponseState).active).toBe(true); + + vi.advanceTimersByTime(600_000); + + expect(res.log.error).toHaveBeenCalledWith(expect.any(Object), expect.stringContaining('[DEFERRED][MAX_DURATION]')); + expect(res._ended()).toBe(true); + + // Headers were locked at 200 when deferring, so the failure has to arrive in the body. + const body = JSON.parse(res._chunks.join('')); + expect(body).toMatchObject({ error: true, success: false }); + expect(body.message).toContain('may still have been applied'); + }); + + it('should measure max duration from when the request arrived, not from when it was deferred', () => { + const req = createMockReq(); + const res = createMockRes(); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + deferredResponseMiddleware(req as any, res as any, vi.fn()); + vi.advanceTimersByTime(75_000); + + // One tick short of the 600s total budget, so the 75s already spent has to have been subtracted. + vi.advanceTimersByTime(524_999); + expect(res._ended()).toBe(false); + + vi.advanceTimersByTime(1); + expect(res._ended()).toBe(true); + }); + + it('should flag the response as abandoned so a late controller response is not reported as unhandled', () => { + const req = createMockReq(); + const res = createMockRes(); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + deferredResponseMiddleware(req as any, res as any, vi.fn()); + vi.advanceTimersByTime(75_000); + expect((res.locals._deferred as DeferredResponseState).abandoned).toBe(false); + + vi.advanceTimersByTime(600_000); + + expect((res.locals._deferred as DeferredResponseState).abandoned).toBe(true); + expect((res.locals._deferred as DeferredResponseState).active).toBe(false); + }); + + it('should not fire once the controller has produced a response', () => { + const req = createMockReq(); + const res = createMockRes(); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + deferredResponseMiddleware(req as any, res as any, vi.fn()); + vi.advanceTimersByTime(75_000); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + writeDeferredResponse(res as any, { data: 'real result' }); + + vi.advanceTimersByTime(600_000); + + expect(res.log.error).not.toHaveBeenCalledWith(expect.any(Object), expect.stringContaining('[DEFERRED][MAX_DURATION]')); + expect(JSON.parse(res._chunks.join(''))).toEqual({ data: 'real result' }); + }); + + it('should stop keepalives so the socket cannot be held indefinitely', () => { + const req = createMockReq(); + const res = createMockRes(); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + deferredResponseMiddleware(req as any, res as any, vi.fn()); + vi.advanceTimersByTime(75_000); + vi.advanceTimersByTime(600_000); + + const chunkCountAfterAbandon = res._chunks.length; + vi.advanceTimersByTime(300_000); + + expect(res._chunks.length).toBe(chunkCountAfterAbandon); + }); +}); + describe('JSON.parse with space-padded response', () => { it('should parse JSON with leading spaces (simulating chunked keepalive)', () => { const originalJson = '{"data":{"message":"hello"}}'; diff --git a/apps/api/src/app/utils/__tests__/error-handler.spec.ts b/apps/api/src/app/utils/__tests__/error-handler.spec.ts new file mode 100644 index 000000000..f8cc3fe33 --- /dev/null +++ b/apps/api/src/app/utils/__tests__/error-handler.spec.ts @@ -0,0 +1,48 @@ +import { ERROR_MESSAGES } from '@jetstream/shared/constants'; +import { describe, expect, it, vi } from 'vitest'; +import { UserFacingError } from '../error-handler'; + +vi.mock('@jetstream/api-config', () => ({ + logger: { warn: vi.fn(), error: vi.fn(), info: vi.fn(), debug: vi.fn() }, +})); + +/** Shape Node/undici produces: an opaque TypeError with the real reason hanging off `cause`. */ +function buildFetchFailure(code?: string) { + const error = new TypeError('fetch failed'); + error.cause = code ? Object.assign(new Error('upstream blew up'), { code }) : undefined; + return error; +} + +describe('UserFacingError upstream fetch failures', () => { + it('replaces an undici headers timeout with copy that does not claim the operation failed', () => { + const error = new UserFacingError(buildFetchFailure('UND_ERR_HEADERS_TIMEOUT')); + + expect(error.message).toBe(ERROR_MESSAGES.SFDC_UPSTREAM_TIMEOUT); + expect(error.message).not.toContain('fetch failed'); + // The whole point: the write may have landed, so the user must check before retrying. + expect(error.message).toContain('may still have been applied'); + }); + + it('treats UND_ERR_BODY_TIMEOUT as a timeout', () => { + expect(new UserFacingError(buildFetchFailure('UND_ERR_BODY_TIMEOUT')).message).toBe(ERROR_MESSAGES.SFDC_UPSTREAM_TIMEOUT); + }); + + // A connect timeout means the request never reached Salesforce, so nothing can have been applied + // and the user should just retry — the opposite of what the timeout copy tells them to do. + it.each(['ECONNRESET', 'UND_ERR_CONNECT_TIMEOUT'])('reports %s as unreachable rather than as a timeout', (code) => { + expect(new UserFacingError(buildFetchFailure(code)).message).toBe(ERROR_MESSAGES.SFDC_UPSTREAM_UNREACHABLE); + }); + + it('reports a fetch failure with no cause as unreachable rather than leaking the raw message', () => { + expect(new UserFacingError(buildFetchFailure()).message).toBe(ERROR_MESSAGES.SFDC_UPSTREAM_UNREACHABLE); + }); + + it('leaves unrelated errors untouched', () => { + expect(new UserFacingError(new Error('Required field missing')).message).toBe('Required field missing'); + expect(new UserFacingError('A plain string message').message).toBe('A plain string message'); + }); + + it('still scrubs raw XML error bodies', () => { + expect(new UserFacingError(new Error('')).message).toBe('An unexpected error has occurred'); + }); +}); diff --git a/apps/api/src/app/utils/deferred-response.middleware.ts b/apps/api/src/app/utils/deferred-response.middleware.ts index fc991b5b8..7aadbfffa 100644 --- a/apps/api/src/app/utils/deferred-response.middleware.ts +++ b/apps/api/src/app/utils/deferred-response.middleware.ts @@ -1,5 +1,5 @@ import { ENV, getLogger } from '@jetstream/api-config'; -import { HTTP } from '@jetstream/shared/constants'; +import { ERROR_MESSAGES, HTTP } from '@jetstream/shared/constants'; import type { Response as ExpressResponse, NextFunction, Request } from 'express'; import type { DeferredResponseState, Response } from '../types/route.types'; import { setCookieHeaders } from './response.handlers'; @@ -9,11 +9,16 @@ export type { DeferredResponseState }; /** * Middleware that prevents Cloudflare 524 timeouts for long-running Salesforce API requests. * - * When a response takes longer than the threshold (default 45s), the middleware begins + * When a response takes longer than the threshold (default 75s), the middleware begins * streaming space characters as chunked keepalive bytes. When the actual response is ready, * it is written to the stream. JSON.parse natively ignores leading whitespace, so Axios * on the client handles this transparently. * + * Keepalives stop DEFERRED_RESPONSE_MAX_DURATION_MS (default 10m) after the request arrived, which + * bounds how long a silent upstream can hold a socket open. That is a backstop, not the normal path — + * it sits well above undici's 300s headersTimeout, which is what actually ends most stalled + * Salesforce calls. + * * NOTE: If compression middleware is re-enabled, chunks may be buffered and not flushed * to Cloudflare in time. This middleware requires compression to be disabled. */ @@ -24,8 +29,10 @@ export function deferredResponseMiddleware(req: Request, res: Response, next: Ne const deferred: DeferredResponseState = { active: false, + abandoned: false, timer: null, keepaliveInterval: null, + maxDurationTimer: null, startTime: Date.now(), keepaliveCount: 0, }; @@ -80,6 +87,41 @@ export function deferredResponseMiddleware(req: Request, res: Response, next: Ne return; } + // Backstop: without this the keepalive loop runs for as long as upstream stays silent, holding + // a socket open long after the client (and Cloudflare) have given up. Salesforce calls made via + // fetch are bounded by undici's 300s headersTimeout, but nothing bounds the other paths. + // The budget is measured from when the request arrived rather than from now, so the configured + // value can be compared directly against Cloudflare and load balancer timeouts. The floor keeps + // a max duration set below the threshold from producing a zero delay. + const maxDurationMs = Number(ENV.DEFERRED_RESPONSE_MAX_DURATION_MS) || 600_000; + deferred.maxDurationTimer = setTimeout( + () => { + if (!deferred.active || res.writableEnded) { + return; + } + getLogger().error( + { + method: req.method, + url: req.originalUrl, + elapsedMs: Date.now() - deferred.startTime, + keepaliveCount: deferred.keepaliveCount, + }, + '[DEFERRED][MAX_DURATION] Upstream never responded, abandoning deferred response', + ); + // Flagged before the write so that when the controller eventually finishes, sendJson and the + // error handler can tell this apart from a genuinely unhandled response. + deferred.abandoned = true; + // Headers were committed as 200 when the response was deferred, so the failure has to be + // reported in the body — same envelope the error handler writes. + writeDeferredResponse(res, { + error: true, + success: false, + message: ERROR_MESSAGES.SFDC_UPSTREAM_TIMEOUT, + }); + }, + Math.max(maxDurationMs - elapsedMs, 1_000), + ); + // Start periodic keepalive deferred.keepaliveInterval = setInterval( () => { @@ -231,6 +273,10 @@ export function clearDeferredTimers(deferred: DeferredResponseState) { clearInterval(deferred.keepaliveInterval); deferred.keepaliveInterval = null; } + if (deferred.maxDurationTimer) { + clearTimeout(deferred.maxDurationTimer); + deferred.maxDurationTimer = null; + } } function cleanupDeferred(deferred: DeferredResponseState) { diff --git a/apps/api/src/app/utils/error-handler.ts b/apps/api/src/app/utils/error-handler.ts index 61c1d707e..fbcdd2d7c 100644 --- a/apps/api/src/app/utils/error-handler.ts +++ b/apps/api/src/app/utils/error-handler.ts @@ -2,8 +2,34 @@ import { logger } from '@jetstream/api-config'; import { StepUpAuthRequiredError } from '@jetstream/auth/server'; import { isPrismaError } from '@jetstream/prisma'; import { ApiRequestError } from '@jetstream/salesforce-api'; +import { ERROR_MESSAGES } from '@jetstream/shared/constants'; import z, { ZodError } from 'zod'; +// undici surfaces the real reason on `error.cause`. These two mean the request reached Salesforce and +// we gave up waiting for the response, so the work may have been done — the distinction that matters +// to the user. Everything else (UND_ERR_CONNECT_TIMEOUT included) failed before Salesforce saw the +// request, and is reported as unreachable so the user can simply retry. +const UPSTREAM_TIMEOUT_CODES = new Set(['UND_ERR_HEADERS_TIMEOUT', 'UND_ERR_BODY_TIMEOUT']); + +/** + * Node's fetch collapses every transport failure into an opaque `TypeError: fetch failed`, which we + * passed straight through to users ("Error saving permissions: fetch failed"). Returns replacement + * copy, or null when this is not a fetch transport failure. + * + * The copy names Salesforce because every raw `fetch failed` that reaches here comes from the + * Salesforce callout layer: the route wrapper turns any unknown controller error into a + * UserFacingError, and the other server-side fetch callers (SAML metadata, OIDC discovery, domain + * verification, geo-IP) either handle their own transport errors or throw a message string, so they + * never hit this branch. Revisit the wording if a non-Salesforce callout starts bubbling raw. + */ +function getUpstreamFetchFailureMessage(error: Error): string | null { + if (error.message !== 'fetch failed') { + return null; + } + const { code } = (error.cause ?? {}) as { code?: string }; + return code && UPSTREAM_TIMEOUT_CODES.has(code) ? ERROR_MESSAGES.SFDC_UPSTREAM_TIMEOUT : ERROR_MESSAGES.SFDC_UPSTREAM_UNREACHABLE; +} + function initStatus(data: unknown, fallback: number) { if (data && typeof data === 'object' && 'status' in data && typeof data.status === 'number') { return data.status; @@ -50,6 +76,11 @@ export class UserFacingError extends Error { logger.warn({ message: message.message }, '[XML ERROR]'); message.message = 'An unexpected error has occurred'; } + const upstreamFailureMessage = getUpstreamFetchFailureMessage(message); + if (upstreamFailureMessage) { + logger.warn({ cause: message.cause }, '[UPSTREAM FETCH FAILURE]'); + message.message = upstreamFailureMessage; + } super(message.message); this.additionalData = additionalData; this.name = message.name; diff --git a/apps/api/src/app/utils/response.handlers.ts b/apps/api/src/app/utils/response.handlers.ts index 176f37f65..3ae438185 100644 --- a/apps/api/src/app/utils/response.handlers.ts +++ b/apps/api/src/app/utils/response.handlers.ts @@ -92,6 +92,15 @@ export function sendJson(res: Response, content?: Respon } if (res.headersSent) { + // The max duration backstop already ended this response and logged why. The late body is the + // expected tail of that, so it is not reported as an unhandled response. + if (deferred?.abandoned) { + getLogger().info( + { elapsedMs: Date.now() - deferred.startTime }, + '[DEFERRED][LATE_BODY] Controller finished after the deferred response was abandoned, discarding body', + ); + return; + } getLogger().warn('Response headers already sent'); try { errorTracker.warn('Response not handled by sendJson, headers already sent', new Error('headers already sent'), { @@ -262,6 +271,15 @@ export async function uncaughtErrorHandler(err: any, req: express.Request, res: } if (res.headersSent) { + // The max duration backstop already ended this response and logged why. The late error is the + // expected tail of that, so it is not reported as an unhandled response. + if (deferred?.abandoned) { + responseLogger.info( + { elapsedMs: Date.now() - deferred.startTime, err }, + '[DEFERRED][LATE_ERROR] Controller failed after the deferred response was abandoned, discarding error', + ); + return; + } responseLogger.warn('Response headers already sent'); try { errorTracker.warn('Error not handled by error handler, headers already sent', req, err, { diff --git a/apps/jetstream-canvas/src/main.tsx b/apps/jetstream-canvas/src/main.tsx index 81472cdc2..4553d3f20 100644 --- a/apps/jetstream-canvas/src/main.tsx +++ b/apps/jetstream-canvas/src/main.tsx @@ -1,4 +1,6 @@ // DO NOT CHANGE ORDER OF IMPORTS +import '@jetstream/shared/utils/configure-zod'; + import { CONFIG } from './app/core/config'; // DO NOT CHANGE ORDER OF IMPORTS diff --git a/apps/jetstream-canvas/vite.config.mts b/apps/jetstream-canvas/vite.config.mts index 79882c75d..0edba40b9 100644 --- a/apps/jetstream-canvas/vite.config.mts +++ b/apps/jetstream-canvas/vite.config.mts @@ -23,6 +23,14 @@ export default defineConfig(() => ({ commonjsOptions: { transformMixedEsModules: true, }, + rolldownOptions: { + output: { + // configure-zod has to be its own chunk. Merged into the entry chunk it would run after every + // chunk the entry imports — including the one that builds schemas, which is too late. + // See libs/shared/utils/src/lib/configure-zod.ts + advancedChunks: { groups: [{ name: 'configure-zod', test: /configure-zod/, priority: 100 }] }, + }, + }, }, define: { 'import.meta.vitest': undefined, diff --git a/apps/jetstream-desktop-client/src/main.tsx b/apps/jetstream-desktop-client/src/main.tsx index ad4bcfe52..07e24543c 100644 --- a/apps/jetstream-desktop-client/src/main.tsx +++ b/apps/jetstream-desktop-client/src/main.tsx @@ -1,4 +1,6 @@ // DO NOT CHANGE ORDER OF IMPORTS +import '@jetstream/shared/utils/configure-zod'; + import { CONFIG } from './app/components/core/config'; // DO NOT CHANGE ORDER OF IMPORTS diff --git a/apps/jetstream-desktop-client/vite.config.mts b/apps/jetstream-desktop-client/vite.config.mts index d455c6a02..c5e653316 100644 --- a/apps/jetstream-desktop-client/vite.config.mts +++ b/apps/jetstream-desktop-client/vite.config.mts @@ -31,6 +31,12 @@ export default defineConfig(() => ({ main: resolve(import.meta.dirname, 'index.html'), // preferences: resolve(import.meta.dirname, 'preferences', 'index.html'), }, + output: { + // configure-zod has to be its own chunk. Merged into the entry chunk it would run after every + // chunk the entry imports — including the one that builds schemas, which is too late. + // See libs/shared/utils/src/lib/configure-zod.ts + advancedChunks: { groups: [{ name: 'configure-zod', test: /configure-zod/, priority: 100 }] }, + }, }, }, plugins: [ diff --git a/apps/jetstream-web-extension/src/pages/additional-settings/AdditionalSettings.tsx b/apps/jetstream-web-extension/src/pages/additional-settings/AdditionalSettings.tsx index 33e4dd16b..0ccfd31f6 100644 --- a/apps/jetstream-web-extension/src/pages/additional-settings/AdditionalSettings.tsx +++ b/apps/jetstream-web-extension/src/pages/additional-settings/AdditionalSettings.tsx @@ -1,3 +1,7 @@ +// Must stay the first import — Zod probes for eval support when the first schema is constructed, +// which happens while these imports are evaluated. See configure-zod. +import '@jetstream/shared/utils/configure-zod'; + import { logger } from '@jetstream/shared/client-logger'; import { AutoFullHeightContainer, diff --git a/apps/jetstream-web-extension/src/pages/app/App.tsx b/apps/jetstream-web-extension/src/pages/app/App.tsx index 72d4f090d..9d961bf1e 100644 --- a/apps/jetstream-web-extension/src/pages/app/App.tsx +++ b/apps/jetstream-web-extension/src/pages/app/App.tsx @@ -1,4 +1,8 @@ /* eslint-disable no-restricted-globals */ +// Must stay the first import — Zod probes for eval support when the first schema is constructed, +// which happens while these imports are evaluated. See configure-zod. +import '@jetstream/shared/utils/configure-zod'; + import { AnonymousApex } from '@jetstream/feature/anon-apex'; import { AutomationControl, AutomationControlEditor, AutomationControlSelection } from '@jetstream/feature/automation-control'; import { CreateFields, CreateFieldsSelection, CreateObjectAndFields } from '@jetstream/feature/create-object-and-fields'; diff --git a/apps/jetstream-web-extension/src/pages/popup/Popup.tsx b/apps/jetstream-web-extension/src/pages/popup/Popup.tsx index c18c5de9d..25536674b 100644 --- a/apps/jetstream-web-extension/src/pages/popup/Popup.tsx +++ b/apps/jetstream-web-extension/src/pages/popup/Popup.tsx @@ -1,3 +1,7 @@ +// Must stay the first import — Zod probes for eval support when the first schema is constructed, +// which happens while these imports are evaluated. See configure-zod. +import '@jetstream/shared/utils/configure-zod'; + import { css } from '@emotion/react'; import { ColorScheme } from '@jetstream/types'; import { CheckboxToggle, FeedbackLink, Grid, ScopedNotification, Select } from '@jetstream/ui'; diff --git a/apps/jetstream-web-extension/vite.config.mts b/apps/jetstream-web-extension/vite.config.mts index d98daf49b..f981cd937 100644 --- a/apps/jetstream-web-extension/vite.config.mts +++ b/apps/jetstream-web-extension/vite.config.mts @@ -66,6 +66,10 @@ export default defineConfig(({ command, mode }) => { entryFileNames: '[name].js', chunkFileNames: '[name]-[hash].js', assetFileNames: '[name].[ext]', + // configure-zod has to be its own chunk. Merged into an entry chunk it would run after every + // chunk that entry imports — including the one that builds schemas, which is too late. + // See libs/shared/utils/src/lib/configure-zod.ts + advancedChunks: { groups: [{ name: 'configure-zod', test: /configure-zod/, priority: 100 }] }, }, }, }, diff --git a/apps/jetstream/src/main.tsx b/apps/jetstream/src/main.tsx index 463761364..5d1480c7b 100644 --- a/apps/jetstream/src/main.tsx +++ b/apps/jetstream/src/main.tsx @@ -1,5 +1,6 @@ /* eslint-disable no-restricted-globals */ // DO NOT CHANGE ORDER OF IMPORTS +import '@jetstream/shared/utils/configure-zod'; import { CONFIG } from './app/components/core/config'; // DO NOT CHANGE ORDER OF IMPORTS diff --git a/apps/jetstream/vite.config.mts b/apps/jetstream/vite.config.mts index c1f550c53..be0941c53 100644 --- a/apps/jetstream/vite.config.mts +++ b/apps/jetstream/vite.config.mts @@ -93,7 +93,14 @@ export default defineConfig(() => ({ assetsDir: './', sourcemap: uploadSourcemaps ? ('hidden' as const) : false, emptyOutDir: true, - rolldownOptions: {}, + rolldownOptions: { + output: { + // configure-zod has to be its own chunk. Merged into the entry chunk it would run after every + // chunk the entry imports — including the one that builds schemas, which is too late. + // See libs/shared/utils/src/lib/configure-zod.ts + advancedChunks: { groups: [{ name: 'configure-zod', test: /configure-zod/, priority: 100 }] }, + }, + }, }, test: { name: 'jetstream', diff --git a/apps/landing/pages/_app.js b/apps/landing/pages/_app.js index d4c291795..7c5b1d13e 100644 --- a/apps/landing/pages/_app.js +++ b/apps/landing/pages/_app.js @@ -1,3 +1,7 @@ +// Must stay the first import — Zod probes for eval support when the first schema is constructed, +// which happens while these imports are evaluated. See configure-zod. +import '@jetstream/shared/utils/configure-zod'; + import { CookieConsentBanner, useConditionalGoogleAnalytics } from '@jetstream/ui/cookie-consent-banner'; import Layout from '../components/layouts/Layout'; import './index.css'; diff --git a/libs/api-config/src/lib/env-config.ts b/libs/api-config/src/lib/env-config.ts index 522df1022..4231a5bbe 100644 --- a/libs/api-config/src/lib/env-config.ts +++ b/libs/api-config/src/lib/env-config.ts @@ -120,9 +120,12 @@ const envSchema = z.object({ IS_LOCAL_DOCKER: booleanSchema, ENABLE_TEST_ENDPOINTS: booleanSchema, DEFERRED_RESPONSE_ENABLED: booleanSchema, - // Defaults are applied in the middleware (45000ms / 25000ms) since numberSchema transforms undefined → null + // Defaults are applied in the middleware (75000ms / 25000ms / 600000ms) since numberSchema transforms undefined → null DEFERRED_RESPONSE_THRESHOLD_MS: numberSchema, DEFERRED_RESPONSE_KEEPALIVE_MS: numberSchema, + // Backstop only — total request duration, including the pre-deferral threshold, after which a + // deferred response gives up on upstream instead of holding the socket open + DEFERRED_RESPONSE_MAX_DURATION_MS: numberSchema, // SYSTEM NODE_ENV: z .enum(['development', 'test', 'production']) diff --git a/libs/shared/constants/src/lib/shared-constants.ts b/libs/shared/constants/src/lib/shared-constants.ts index a194d5c9c..8889c6717 100644 --- a/libs/shared/constants/src/lib/shared-constants.ts +++ b/libs/shared/constants/src/lib/shared-constants.ts @@ -104,6 +104,11 @@ export const ERROR_MESSAGES = { SFDC_ORG_DOES_NOT_EXIST: /^getaddrinfo ENOTFOUND [a-z0-9-.]+\.salesforce\.com$/i, SFDC_REST_API_NOT_ENABLED: /api is not enabled/i, SFDC_REST_API_NOT_ENABLED_MSG: 'Your org/user does not have API access which is required for Jetstream to communicate with Salesforce.', + // Node's fetch collapses every transport failure into an opaque "fetch failed", which used to reach + // users verbatim. These replace it. The timeout wording deliberately avoids saying the operation + // failed — the request did reach Salesforce, so it may well have been applied with the response lost. + SFDC_UPSTREAM_TIMEOUT: 'Salesforce did not respond in time. The operation may still have been applied — reload to check before retrying.', + SFDC_UPSTREAM_UNREACHABLE: 'Unable to reach Salesforce. Check your connection and try again.', } as const; export const MIME_TYPES: { diff --git a/libs/shared/ui-utils/src/lib/__tests__/errorTracker.spec.ts b/libs/shared/ui-utils/src/lib/__tests__/errorTracker.spec.ts new file mode 100644 index 000000000..d46b00627 --- /dev/null +++ b/libs/shared/ui-utils/src/lib/__tests__/errorTracker.spec.ts @@ -0,0 +1,92 @@ +import type * as Sentry from '@sentry/react'; +import { describe, expect, it } from 'vitest'; +import { shouldIgnore } from '../errorTracker'; + +interface EventOptions { + type?: string; + value?: string; + filenames?: string[]; +} + +function buildEvent({ type = 'Error', value, filenames = [] }: EventOptions): Sentry.ErrorEvent { + return { + exception: { + values: [ + { + type, + value, + stacktrace: { frames: filenames.map((filename) => ({ filename })) }, + }, + ], + }, + } as Sentry.ErrorEvent; +} + +const MONACO_BLOB_URL = 'blob:https://getjetstream.app/c7424921-6d50-4125-85d7-b0a71bd58fb8'; +const SENTRY_INTERNAL_FRAME = '../../../node_modules/.pnpm/@sentry+core@10.69.0/node_modules/@sentry/core/build/esm/exports.js'; + +describe('shouldIgnore', () => { + describe('monaco blob: worker script load failures', () => { + it('ignores the raw Firefox NetworkError thrown from the blob: worker bootstrap', () => { + const event = buildEvent({ value: 'NetworkError: A network error occurred.', filenames: [MONACO_BLOB_URL] }); + expect(shouldIgnore(event)).toBe(true); + }); + + it('ignores the worker ErrorEvent that Sentry wraps, whose frames are all Sentry internals', () => { + const event = buildEvent({ + type: 'ErrorEvent', + value: 'Event `ErrorEvent` captured as exception with message `NetworkError: A network error occurred.`', + filenames: [SENTRY_INTERNAL_FRAME], + }); + expect(shouldIgnore(event)).toBe(true); + }); + + it('ignores the Chromium wording for a failed importScripts', () => { + const event = buildEvent({ + value: "Failed to execute 'importScripts' on 'WorkerGlobalScope': The script at 'https://getjetstream.app/x.js' failed to load.", + filenames: [MONACO_BLOB_URL], + }); + expect(shouldIgnore(event)).toBe(true); + }); + + // The whole point of requiring a blob: frame — Firefox words a failed app fetch identically, and + // those are real errors we still want to see. + it('reports an identically worded NetworkError raised from app code', () => { + const event = buildEvent({ + value: 'NetworkError: A network error occurred.', + filenames: ['https://getjetstream.app/src-D6zgEThK.js'], + }); + expect(shouldIgnore(event)).toBe(false); + }); + + it('reports an unrelated error that happens to originate in a blob: worker', () => { + const event = buildEvent({ value: 'Cannot read properties of undefined', filenames: [MONACO_BLOB_URL] }); + expect(shouldIgnore(event)).toBe(false); + }); + }); + + describe('pre-existing rules', () => { + it('ignores errors from monaco assets', () => { + const event = buildEvent({ + value: 'Some monaco explosion', + filenames: ['https://getjetstream.app/assets/js/monaco/vs/editor.main.js'], + }); + expect(shouldIgnore(event)).toBe(true); + }); + + it('ignores errors originating in browser extensions', () => { + const event = buildEvent({ value: 'Some extension explosion', filenames: ['chrome-extension://abc/content.js'] }); + expect(shouldIgnore(event)).toBe(true); + }); + + it('ignores known non-actionable messages', () => { + expect(shouldIgnore(buildEvent({ value: 'socket hang up' }))).toBe(true); + expect(shouldIgnore(buildEvent({ type: 'DatabaseClosedError' }))).toBe(true); + }); + + it('reports a genuine application error', () => { + const event = buildEvent({ value: 'Cannot read properties of undefined', filenames: ['https://getjetstream.app/src-D6zgEThK.js'] }); + expect(shouldIgnore(event)).toBe(false); + }); + }); +}); diff --git a/libs/shared/ui-utils/src/lib/errorTracker.ts b/libs/shared/ui-utils/src/lib/errorTracker.ts index 53f14e7a9..c282b46cd 100644 --- a/libs/shared/ui-utils/src/lib/errorTracker.ts +++ b/libs/shared/ui-utils/src/lib/errorTracker.ts @@ -32,6 +32,13 @@ const ignoredMessagePatterns = [INVALID_QUERY_LOCATOR_REGEX]; const ignoredExactMessages = new Set(['Canceled', 'ChunkLoadError', '(unknown)', 'DatabaseClosedError']); const extensionUrlPrefixes = ['chrome-extension://', 'moz-extension://', 'safari-web-extension://', 'safari-extension://']; +// How browsers word a worker whose script could not be fetched: Firefox reports a bare NetworkError, +// Chromium names importScripts explicitly. +const workerScriptLoadFailurePattern = /^NetworkError: A network error occurred\.$|Failed to execute 'importScripts'/; +// A worker reports failure as an ErrorEvent, which Sentry wraps rather than reporting directly. The +// real message is quoted inside. +const wrappedErrorEventPattern = /^Event `ErrorEvent` captured as exception with message `(.+)`$/; + const PER_SESSION_MINUTE_LIMIT = 10; const PER_SESSION_TOTAL_LIMIT = 20; @@ -58,7 +65,8 @@ function isRateLimited(): boolean { return false; } -function shouldIgnore(event: Sentry.ErrorEvent): boolean { +/** Exported for testing — the ignore rules are the only thing standing between us and alert storms. */ +export function shouldIgnore(event: Sentry.ErrorEvent): boolean { const candidates: string[] = []; if (event.message) { candidates.push(event.message); @@ -89,9 +97,42 @@ function shouldIgnore(event: Sentry.ErrorEvent): boolean { if (allFrames.some((frame) => extensionUrlPrefixes.some((prefix) => frame.filename?.startsWith(prefix)))) { return true; } + if ( + isWorkerScriptLoadFailure( + candidates, + allFrames.some((frame) => frame.filename?.startsWith('blob:')), + ) + ) { + return true; + } return false; } +/** + * Monaco builds its editor workers from a `blob:` bootstrap that `importScripts()` the real worker + * file. When that fetch is blocked (browser extension, tracking protection, corporate proxy) the + * failure arrives in two shapes, neither of which the `/js/monaco/vs/` rule can match: the raw error, + * whose only stack frame is the blob URL, and the worker's ErrorEvent, whose frames are all Sentry + * internals. Monaco catches this itself and falls back to running worker code on the main thread, so + * neither is actionable. + * + * Filtering here is the only thing that stops the noise: every occurrence carries a fresh blob UUID, + * so error tracking groups each one as a brand-new error and re-alerts. Marking them resolved + * upstream does nothing. + */ +function isWorkerScriptLoadFailure(candidates: string[], hasBlobFrame: boolean): boolean { + return candidates.some((candidate) => { + const wrappedMessage = wrappedErrorEventPattern.exec(candidate)?.[1]; + if (!workerScriptLoadFailurePattern.test(wrappedMessage ?? candidate)) { + return false; + } + // An ErrorEvent means the failure came from a worker rather than app code, which is attribution + // enough on its own. A raw error needs the blob: frame, so a failed `fetch` in app code — which + // Firefox words identically — still gets reported. + return wrappedMessage !== undefined || hasBlobFrame; + }); +} + /** * Initialize the error tracker. Safe to call multiple times — only the first call with a non-empty dsn does anything. * Call this once at app boot (e.g. AppInitializer) before any errors need to be reported. diff --git a/libs/shared/utils/src/lib/__tests__/configure-zod-timing.spec.ts b/libs/shared/utils/src/lib/__tests__/configure-zod-timing.spec.ts new file mode 100644 index 000000000..2f9dcd919 --- /dev/null +++ b/libs/shared/utils/src/lib/__tests__/configure-zod-timing.spec.ts @@ -0,0 +1,46 @@ +import { describe, expect, it, vi } from 'vitest'; +import { z } from 'zod'; +import { countEvalConstructions, hasProbedForEval } from './zod-eval-probe.utils'; + +/** + * Companion to configure-zod.spec.ts, deliberately kept in its own file: Zod memoizes its eval probe + * the first time it is needed, so a process only ever gets one chance to observe it. These cases need + * a Zod that has not been configured yet, which the other file has already used up. + * + * Together the two files pin the assumption the whole approach rests on — that the probe fires when a + * schema is *constructed*, not when one is parsed — so a Zod upgrade that moves it fails loudly here + * instead of silently restoring thousands of daily CSP reports. + */ +describe('Zod eval probe timing', () => { + it('should probe when a schema is constructed, without anything being parsed', () => { + expect(hasProbedForEval()).toBe(false); + + const evalConstructions = countEvalConstructions(() => { + z.object({ name: z.string() }); + }); + + expect(evalConstructions).toBe(1); + expect(hasProbedForEval()).toBe(true); + }); + + it('should not be undone by configuring Zod after a schema has been constructed', async () => { + // Ordering mirrors an entry point that calls into configure-zod from its own body: every module it + // imported — @jetstream/types included — has already built its schemas by then. + await import('../configure-zod'); + + // The result is memoized, so the `new Function` call (and the CSP violation it reports) has already + // happened. Late configuration silences nothing, which is why this is a first-position import. + expect(hasProbedForEval()).toBe(true); + }); + + it('should leave the JIT path alone outside the browser', async () => { + z.config({ jitless: false }); + vi.stubGlobal('window', undefined); + vi.resetModules(); + + await import('../configure-zod'); + + expect(z.config().jitless).toBe(false); + vi.unstubAllGlobals(); + }); +}); diff --git a/libs/shared/utils/src/lib/__tests__/configure-zod.spec.ts b/libs/shared/utils/src/lib/__tests__/configure-zod.spec.ts new file mode 100644 index 000000000..fb61ec215 --- /dev/null +++ b/libs/shared/utils/src/lib/__tests__/configure-zod.spec.ts @@ -0,0 +1,66 @@ +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { z } from 'zod'; +import { countEvalConstructions, hasProbedForEval } from './zod-eval-probe.utils'; + +const REPO_ROOT = join(import.meta.dirname, '../../../../../..'); + +/** + * Every browser entry point, all of which must import configure-zod before anything else. Zod's eval + * probe runs while an entry point's *imports* are evaluated — long before its own first statement — + * so import position is the entire contract. + */ +const BROWSER_ENTRY_POINTS = [ + 'apps/jetstream/src/main.tsx', + 'apps/jetstream-desktop-client/src/main.tsx', + 'apps/jetstream-canvas/src/main.tsx', + 'apps/landing/pages/_app.js', + 'apps/jetstream-web-extension/src/pages/app/App.tsx', + 'apps/jetstream-web-extension/src/pages/popup/Popup.tsx', + 'apps/jetstream-web-extension/src/pages/additional-settings/AdditionalSettings.tsx', +]; + +/** + * The other half of the contract. In a Vite build, being first in the entry file is not enough: unless + * configure-zod gets its own chunk it is merged into the entry chunk, which runs after every chunk the + * entry imports — schemas included. Verified against the production builds under a strict CSP. + */ +const VITE_APP_CONFIGS = [ + 'apps/jetstream/vite.config.mts', + 'apps/jetstream-desktop-client/vite.config.mts', + 'apps/jetstream-canvas/vite.config.mts', + 'apps/jetstream-web-extension/vite.config.mts', +]; + +describe('configure-zod', () => { + it('should keep Zod from probing for eval support when schemas are built and parsed', async () => { + // Imported here rather than at the top of the file so the ordering the test depends on — configured + // before the first schema is constructed — is explicit and survives organize-imports. + await import('../configure-zod'); + + const evalConstructions = countEvalConstructions(() => { + const schema = z.object({ name: z.string() }); + schema.parse({ name: 'Jetstream' }); + }); + + // Zero, not "one that fails" — under a strict CSP a caught failure is still a reported violation. + expect(evalConstructions).toBe(0); + expect(hasProbedForEval()).toBe(false); + expect(z.config().jitless).toBe(true); + }); + + it.each(BROWSER_ENTRY_POINTS)('should be the first import in %s', (entryPoint) => { + const firstImport = readFileSync(join(REPO_ROOT, entryPoint), 'utf8') + .split('\n') + .find((line) => line.startsWith('import')); + + expect(firstImport).toBe(`import '@jetstream/shared/utils/configure-zod';`); + }); + + it.each(VITE_APP_CONFIGS)('should be given its own chunk by %s', (viteConfig) => { + expect(readFileSync(join(REPO_ROOT, viteConfig), 'utf8')).toContain( + `advancedChunks: { groups: [{ name: 'configure-zod', test: /configure-zod/, priority: 100 }] }`, + ); + }); +}); diff --git a/libs/shared/utils/src/lib/__tests__/zod-eval-probe.utils.ts b/libs/shared/utils/src/lib/__tests__/zod-eval-probe.utils.ts new file mode 100644 index 000000000..389e728ec --- /dev/null +++ b/libs/shared/utils/src/lib/__tests__/zod-eval-probe.utils.ts @@ -0,0 +1,33 @@ +import { util } from 'zod/v4/core'; + +/** + * Zod decides whether it can JIT-compile validators by running `new Function('')` once and memoizing + * the answer. In the browser that single call is what a strict CSP reports as a `script-src eval` + * violation, so these helpers observe exactly that call rather than any Zod-internal flag. + */ + +/** Counts `new Function(...)` calls made while `runSchemaWork` executes. */ +export function countEvalConstructions(runSchemaWork: () => void): number { + const OriginalFunction = globalThis.Function; + let evalConstructions = 0; + globalThis.Function = new Proxy(OriginalFunction, { + construct: (target, argumentList, newTarget) => { + evalConstructions++; + return Reflect.construct(target, argumentList, newTarget); + }, + }); + try { + runSchemaWork(); + } finally { + globalThis.Function = OriginalFunction; + } + return evalConstructions; +} + +/** + * Zod memoizes the probe by replacing its lazy getter with a plain value, so the getter still being + * in place proves the probe never ran. + */ +export function hasProbedForEval(): boolean { + return Object.getOwnPropertyDescriptor(util.allowsEval, 'value')?.get === undefined; +} diff --git a/libs/shared/utils/src/lib/configure-zod.ts b/libs/shared/utils/src/lib/configure-zod.ts new file mode 100644 index 000000000..e019e2e76 --- /dev/null +++ b/libs/shared/utils/src/lib/configure-zod.ts @@ -0,0 +1,37 @@ +import { z } from 'zod'; + +/** + * Side-effect module — importing it disables Zod's JIT compilation in the browser. + * + * Zod probes for `new Function` support to decide whether it can JIT-compile validators. Under our + * browser CSP (`script-src` has no `'unsafe-eval'`) that probe always fails, and Zod swallows the + * throw — but the browser still reports a `script-src eval` violation for every page load. That was + * ~3k reports a day to `/api/csp-report`, drowning out any real violation. + * + * Setting `jitless` short-circuits the probe before it runs. There is no runtime cost in the browser: + * the CSP had already forced Zod onto the interpreted path, so this only stops the reporting noise. + * + * WHY THIS IS A SIDE-EFFECT MODULE, AND WHY IT NEEDS BUILD CONFIG + * + * Zod runs the probe (and memoizes the result) the first time a schema is **constructed** — the + * `z.object({...})` that runs at module scope in `@jetstream/types` and friends — not the first time + * one is parsed. Winning that race takes two things, and both are load-bearing: + * + * 1. This module must be the FIRST import of the entry point. ES modules evaluate every import + * before the importing module's own body, so a function called from an entry point's body always + * loses. + * 2. Every Vite app must keep the `configure-zod` chunk group in its build config. Position in the + * entry file is not enough on its own: merged into the entry chunk, this code runs after every + * chunk the entry imports — including the one holding the schemas. Its own chunk is what makes + * the entry import (and run) it first. This was verified by loading the production builds under a + * production-like CSP; without the group, the violation is still reported. + * + * Next.js (landing) needs only the import — webpack's chunking already preserves the order there. + * configure-zod.spec.ts pins both requirements. + * + * No-ops outside the browser so the server (and Next.js static generation) keeps the JIT path, where + * it works and is worth having. + */ +if (typeof window !== 'undefined') { + z.config({ jitless: true }); +} diff --git a/tsconfig.base.json b/tsconfig.base.json index decff73e3..4b0335048 100644 --- a/tsconfig.base.json +++ b/tsconfig.base.json @@ -63,6 +63,7 @@ "@jetstream/shared/ui-router": ["./libs/shared/ui-router/src/index.ts"], "@jetstream/shared/ui-utils": ["./libs/shared/ui-utils/src/index.ts"], "@jetstream/shared/utils": ["./libs/shared/utils/src/index.ts"], + "@jetstream/shared/utils/configure-zod": ["./libs/shared/utils/src/lib/configure-zod.ts"], "@jetstream/splitjs": ["./libs/splitjs/src/index.ts"], "@jetstream/test-utils": ["./libs/test-utils/src/index.ts"], "@jetstream/test/e2e-utils": ["./libs/test/e2e-utils/src/index.ts"],