diff --git a/.github/workflows/e2e-stack.yml b/.github/workflows/e2e-stack.yml new file mode 100644 index 0000000000..4ffde933d5 --- /dev/null +++ b/.github/workflows/e2e-stack.yml @@ -0,0 +1,175 @@ +# Full-stack E2E: build this PR's API image and run the Playwright harness from +# the public companion services repo (e2e-stack/) against it. +# +# Do NOT add a job-level `if:` skip condition. A skipped GitHub Actions check +# counts as passing, which would silently defeat the merge gate. Same reasoning +# as the coverage-gate job in api-pr.yaml. + +name: Full-stack E2E + +on: + pull_request: + branches: + - main + - develop + workflow_dispatch: + inputs: + # Deliberately without a default. An empty value means "develop, and bootstrap from the + # pull request that introduces the harness if develop does not have it yet"; a value the + # caller typed means "use exactly this ref, and fail if the harness is not on it". A + # default of `develop` would make those two indistinguishable and turn every manual run + # into the strict variant. + services_ref: + description: Ref of DFXswiss/services holding the e2e-stack harness (empty = develop) + required: false + type: string + +permissions: + contents: read + +concurrency: + group: e2e-stack-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + e2e: + name: Full-stack E2E + runs-on: ubuntu-latest + timeout-minutes: 45 + steps: + - name: Checkout API (this repo) + uses: actions/checkout@v4 + with: + path: api-repo + + - name: Checkout services (e2e-stack harness) + uses: actions/checkout@v4 + with: + repository: DFXswiss/services + ref: ${{ inputs.services_ref || 'develop' }} + path: services-repo + + # Resolve whether the harness is already on the checked-out ref, or whether + # we need the temporary bootstrap from the services PR that introduces it. + # An explicit services_ref that lacks the harness is a hard error (caller + # mistake); only the default develop path may fall through to bootstrap. + - name: Resolve harness location + id: harness + env: + SERVICES_REF: ${{ inputs.services_ref }} + GH_TOKEN: ${{ github.token }} + run: | + set -euo pipefail + if [ -x services-repo/e2e-stack/scripts/up.sh ]; then + echo "bootstrap=false" >> "$GITHUB_OUTPUT" + elif [ -n "$SERVICES_REF" ]; then + echo "::error::The e2e-stack harness is not present in DFXswiss/services@${SERVICES_REF}." + echo "::error::services_ref was set explicitly, so bootstrap is not applied. Point it at a" + echo "::error::ref that contains e2e-stack/, or omit services_ref to use develop (with fallback)." + exit 1 + else + # Capture stderr separately so a harmless gh warning cannot pollute $state + # and make an open PR look closed in the comparison below. + err_file="$(mktemp -p "${RUNNER_TEMP}")" + if ! state="$(gh api repos/DFXswiss/services/pulls/1288 --jq .state 2>"$err_file")"; then + echo "::error::Could not determine the state of DFXswiss/services#1288 (gh api call failed):" + echo "::error::$(cat "$err_file")" + rm -f "$err_file" + exit 1 + fi + rm -f "$err_file" + if [ -z "$state" ]; then + echo "::error::Could not determine the state of DFXswiss/services#1288: gh api returned an empty state." + exit 1 + fi + if [ "$state" != "open" ]; then + echo "::error::DFXswiss/services#1288 is no longer open (state: ${state}); the bootstrap fallback is spent." + echo "::error::The harness is expected on DFXswiss/services@develop now. Delete this bootstrap block" + echo "::error::(Resolve harness location's else branch and the 'Check out the harness from the pull" + echo "::error::request that introduces it' step below) from this workflow." + exit 1 + fi + echo "::warning::The e2e-stack harness is not yet on DFXswiss/services@develop." + echo "::warning::Bootstrapping from the pull request head that introduces it (refs/pull/1288/head)." + echo "::warning::Merge DFXswiss/services#1288 into develop to remove this temporary fallback." + echo "bootstrap=true" >> "$GITHUB_OUTPUT" + fi + + # Temporary bootstrap while the harness lives only on services#1288: check out + # that PR head when develop still lacks e2e-stack/. Self-disabling — once the + # harness lands on develop, Resolve harness location sets bootstrap=false and + # this step is skipped. After services#1288 is merged or closed, the next run of + # Resolve harness location fails loud once it sees a non-open state, instead of + # bootstrapping. There is a small, harmless window between that check and this + # checkout in which the PR could still be merged; the checkout would then fetch + # the same commit that was open a moment earlier, because refs/pull/1288/head + # continues to point at that commit after merge (and stays fetchable on GitHub + # forever). + - name: Check out the harness from the pull request that introduces it + if: steps.harness.outputs.bootstrap == 'true' + uses: actions/checkout@v4 + with: + repository: DFXswiss/services + ref: refs/pull/1288/head + path: services-repo + + - name: Verify the harness is present + run: | + if [ ! -x services-repo/e2e-stack/scripts/up.sh ]; then + echo "::error::The e2e-stack harness is still missing at services-repo/e2e-stack/scripts/up.sh" + echo "::error::after checkout (including the bootstrap fallback from refs/pull/1288/head)." + echo "::error::Confirm that DFXswiss/services#1288 still contains e2e-stack/ and is reachable." + exit 1 + fi + + # No Node setup here on purpose: the harness scripts are Bash plus Docker, and every Node + # dependency is installed inside the images they build. Nothing in this job runs node, npm or + # npx on the runner itself. + + - name: Build API image from PR head + env: + GIT_SHA: ${{ github.sha }} + run: docker build -t dfx-api:e2e --build-arg GIT_COMMIT="$GIT_SHA" api-repo + + # Do not use services-repo/e2e-stack/scripts/run.sh here. That script tears the + # whole stack down (including named volumes) via trap … EXIT before artifacts + # can be copied out. Artifacts live on named volumes under /work/test-results and + # /work/playwright-report; bind mounts are forbidden. Instead: up.sh, then a + # separate compose test run, then artifact copy (always), then down.sh (always). + - name: Bring up e2e stack + run: E2E_API_IMAGE=dfx-api:e2e bash services-repo/e2e-stack/scripts/up.sh + + # --env-file is not optional here: up.sh writes the values it resolved into that file, and a + # compose run that resolves them differently recreates the API container mid-run. + - name: Run Playwright tests + env: + # Every spec is in scope here, so the coverage gate checks navigations, not just claims. + E2E_FULL_RUN: '1' + run: | + docker compose -p dfx-e2e-stack \ + --env-file services-repo/e2e-stack/.env.generated \ + -f services-repo/e2e-stack/compose.yml \ + -f services-repo/e2e-stack/compose.tests.yml \ + run --name dfx-e2e-stack-tests tests + + - name: Collect test artifacts + if: always() + run: | + mkdir -p e2e-artifacts + # Named volumes back these paths; copy out before down.sh removes volumes. + # Container may be stopped (compose run without --rm keeps it for docker cp). + docker cp dfx-e2e-stack-tests:/work/test-results e2e-artifacts/test-results 2>/dev/null || true + docker cp dfx-e2e-stack-tests:/work/playwright-report e2e-artifacts/playwright-report 2>/dev/null || true + + - name: Upload e2e report + if: always() + uses: actions/upload-artifact@v7 + with: + name: e2e-stack-report + path: e2e-artifacts/ + retention-days: 7 + if-no-files-found: warn + + - name: Tear down e2e stack + if: always() + run: bash services-repo/e2e-stack/scripts/down.sh diff --git a/src/integration/blockchain/arbitrum/arbitrum-client.ts b/src/integration/blockchain/arbitrum/arbitrum-client.ts index ac499922b5..8362c5839d 100644 --- a/src/integration/blockchain/arbitrum/arbitrum-client.ts +++ b/src/integration/blockchain/arbitrum/arbitrum-client.ts @@ -19,6 +19,7 @@ import { Util } from 'src/shared/utils/util'; import { EvmClient, EvmClientParams } from '../shared/evm/evm-client'; import { EvmUtil } from '../shared/evm/evm.util'; import { L2BridgeEvmClient } from '../shared/evm/interfaces'; +import { safeLogError } from 'src/shared/utils/safe-log'; export class ArbitrumClient extends EvmClient implements L2BridgeEvmClient { protected override readonly logger = new DfxLogger(ArbitrumClient); @@ -36,7 +37,12 @@ export class ArbitrumClient extends EvmClient implements L2BridgeEvmClient { this.l1Provider = new ethers.providers.JsonRpcProvider(ethereumGateway); this.l1Wallet = new ethers.Wallet(ethWalletPrivateKey, this.l1Provider); - void this.initL2Network(); + // initL2Network() currently handles its own failures, so this catch never fires today. It is + // here because a floating promise in a constructor takes the whole process down the moment + // that stops being true — which is exactly what happened on the Polygon side. + // safeLogError, not logger.error: see the same call on the Polygon client — a rejection value + // whose stack getter throws would turn this catch into a new unhandled rejection. + void this.initL2Network().catch((e) => safeLogError(this.logger, 'Arbitrum L2 network initialization failed:', e)); } async depositCoinOnDex(amount: number): Promise { diff --git a/src/integration/blockchain/polygon/polygon-client.ts b/src/integration/blockchain/polygon/polygon-client.ts index c66a73681a..45ecd345bb 100644 --- a/src/integration/blockchain/polygon/polygon-client.ts +++ b/src/integration/blockchain/polygon/polygon-client.ts @@ -4,6 +4,7 @@ import { Contract, ethers } from 'ethers'; import { Config, GetConfig } from 'src/config/config'; import { Asset } from 'src/shared/models/asset/asset.entity'; import { DfxLogger } from 'src/shared/services/dfx-logger'; +import { safeLogError } from 'src/shared/utils/safe-log'; import { Util } from 'src/shared/utils/util'; import ERC20_ABI from '../shared/evm/abi/erc20.abi.json'; import { EvmClient, EvmClientParams } from '../shared/evm/evm-client'; @@ -33,7 +34,12 @@ export class PolygonClient extends EvmClient implements L2BridgeEvmClient { const { polygonWalletAddress } = GetConfig().blockchain.polygon; this.posClient = new POSClient(); - void this.initPolygonNetwork(ethWalletAddress, polygonWalletAddress); + // safeLogError, not logger.error: the callback runs on a rejection value it did not produce, + // and a value whose stack getter throws would make the callback itself reject - a fresh + // unhandled rejection out of the very catch that exists to prevent one. + void this.initPolygonNetwork(ethWalletAddress, polygonWalletAddress).catch((e) => + safeLogError(this.logger, 'Polygon L2 network initialization failed:', e), + ); this.l2TxIdCache = new Set(); } diff --git a/src/main.ts b/src/main.ts index f4c4b1a5b4..e2dba262a0 100644 --- a/src/main.ts +++ b/src/main.ts @@ -17,6 +17,7 @@ import helmet from 'helmet'; import morgan from 'morgan'; import { join } from 'path'; import { getVerifiedIp } from './shared/utils/ip.util'; +import { handleUncaughtException, handleUnhandledRejection } from './shared/utils/process-error-handlers'; import { AppModule } from './app.module'; import { Config, Environment } from './config/config'; import { ApiExceptionFilter } from './shared/filters/exception.filter'; @@ -32,20 +33,12 @@ import { import { PaymentWebhookDto } from './subdomains/generic/user/services/webhook/dto/payment-webhook.dto'; import { PricingService } from './subdomains/supporting/pricing/services/pricing.service'; -process.on('uncaughtException', (error) => { - const logger = new DfxLogger('UncaughtException'); - - const isSparkError = - error?.constructor?.name?.includes('Spark') || error?.message?.includes('Channel has been shut down'); - - if (isSparkError) { - logger.error('Spark SDK uncaught exception (process kept alive):', error); - return; - } - - logger.error('Uncaught exception, shutting down:', error); - process.exit(1); -}); +process.on('uncaughtException', (error) => + handleUncaughtException(error, { logger: new DfxLogger('UncaughtException'), exit: () => process.exit(1) }), +); +process.on('unhandledRejection', (reason) => + handleUnhandledRejection(reason, { logger: new DfxLogger('UnhandledRejection'), exit: () => process.exit(1) }), +); async function bootstrap() { // Observability is initialized in src/tracing.ts (imported above): the diff --git a/src/shared/utils/__tests__/process-error-handlers.spec.ts b/src/shared/utils/__tests__/process-error-handlers.spec.ts new file mode 100644 index 0000000000..3f1c677376 --- /dev/null +++ b/src/shared/utils/__tests__/process-error-handlers.spec.ts @@ -0,0 +1,300 @@ +import { DfxLogger } from '../../services/dfx-logger'; +import { handleUncaughtException, handleUnhandledRejection, ProcessErrorHandlerDeps } from '../process-error-handlers'; + +function createDeps(overrides?: { errorFn?: jest.Mock; exitFn?: jest.Mock }): { + deps: ProcessErrorHandlerDeps; + errorFn: jest.Mock; + exitFn: jest.Mock; +} { + const errorFn = overrides?.errorFn ?? jest.fn(); + const exitFn = overrides?.exitFn ?? jest.fn(); + const deps: ProcessErrorHandlerDeps = { + logger: { error: errorFn } as unknown as DfxLogger, + exit: exitFn, + }; + return { deps, errorFn, exitFn }; +} + +function createHostileProxy(): object { + return new Proxy( + {}, + { + getPrototypeOf() { + throw new Error('hostile'); + }, + }, + ); +} + +function createUnstringifiable(): object { + return { + [Symbol.toPrimitive](): string { + throw new Error('cannot convert'); + }, + toString(): string { + throw new Error('cannot convert'); + }, + valueOf(): string { + throw new Error('cannot convert'); + }, + }; +} + +class SparkNetworkError extends Error {} + +describe('handleUncaughtException', () => { + it('keeps the process alive for a tolerated Spark constructor name', () => { + const { deps, errorFn, exitFn } = createDeps(); + const error = new SparkNetworkError('network blip'); + + handleUncaughtException(error, deps); + + expect(exitFn).not.toHaveBeenCalled(); + expect(errorFn).toHaveBeenCalledTimes(1); + expect(errorFn).toHaveBeenCalledWith('Spark SDK uncaught exception (process kept alive):', error); + }); + + it('keeps the process alive for a tolerated Channel has been shut down message', () => { + const { deps, exitFn } = createDeps(); + const error = new Error('Channel has been shut down'); + + handleUncaughtException(error, deps); + + expect(exitFn).not.toHaveBeenCalled(); + }); + + it('exits once for a non-tolerated error', () => { + const { deps, errorFn, exitFn } = createDeps(); + const error = new Error('boom'); + + handleUncaughtException(error, deps); + + expect(exitFn).toHaveBeenCalledTimes(1); + expect(errorFn).toHaveBeenCalledTimes(1); + expect(errorFn).toHaveBeenCalledWith('Uncaught exception, shutting down:', error); + }); + + it('does not throw on a hostile Proxy and still exits when not tolerated', () => { + const { deps, exitFn } = createDeps(); + const hostile = createHostileProxy(); + + expect(() => handleUncaughtException(hostile, deps)).not.toThrow(); + expect(exitFn).toHaveBeenCalledTimes(1); + }); + + it('does not throw when logger.error always throws and still exits for a non-tolerated error', () => { + const errorFn = jest.fn(() => { + throw new Error('logger is broken'); + }); + const { deps, exitFn } = createDeps({ errorFn }); + + expect(() => handleUncaughtException(new Error('boom'), deps)).not.toThrow(); + expect(exitFn).toHaveBeenCalledTimes(1); + }); + + it('does not throw when logger.error always throws for a tolerated error and does not exit', () => { + const errorFn = jest.fn(() => { + throw new Error('logger is broken'); + }); + const { deps, exitFn } = createDeps({ errorFn }); + + expect(() => handleUncaughtException(new SparkNetworkError('network blip'), deps)).not.toThrow(); + expect(exitFn).not.toHaveBeenCalled(); + }); + + it('does not throw and exits for undefined', () => { + const { deps, exitFn } = createDeps(); + + expect(() => handleUncaughtException(undefined, deps)).not.toThrow(); + expect(exitFn).toHaveBeenCalledTimes(1); + }); + + it('does not throw and exits for null', () => { + const { deps, exitFn } = createDeps(); + + expect(() => handleUncaughtException(null, deps)).not.toThrow(); + expect(exitFn).toHaveBeenCalledTimes(1); + }); + + it('does not throw and exits for a plain string', () => { + const { deps, exitFn } = createDeps(); + + expect(() => handleUncaughtException('plain failure', deps)).not.toThrow(); + expect(exitFn).toHaveBeenCalledTimes(1); + }); + + it('logs a non-Error exception with its value, distinguishable from a rejection', () => { + const { deps, errorFn, exitFn } = createDeps(); + + handleUncaughtException('plain failure', deps); + + const logged = errorFn.mock.calls[0][1] as Error; + expect(logged).toBeInstanceOf(Error); + expect(logged.message).toBe('Non-Error exception: plain failure'); + expect(exitFn).toHaveBeenCalledTimes(1); + }); + + it('names the value it could not stringify without calling it a rejection', () => { + const { deps, errorFn } = createDeps(); + + handleUncaughtException(createUnstringifiable(), deps); + + const logged = errorFn.mock.calls[0][1] as Error; + expect(logged.message).toBe('Non-Error exception: '); + }); + + it('does not throw and exits for a number', () => { + const { deps, exitFn } = createDeps(); + + expect(() => handleUncaughtException(42, deps)).not.toThrow(); + expect(exitFn).toHaveBeenCalledTimes(1); + }); + + it('does not throw and exits for an object without a prototype', () => { + const { deps, exitFn } = createDeps(); + const value = Object.create(null) as Record; + + expect(() => handleUncaughtException(value, deps)).not.toThrow(); + expect(exitFn).toHaveBeenCalledTimes(1); + }); + + it('does not throw and exits for a null-prototype object with a non-tolerating message', () => { + const { deps, exitFn } = createDeps(); + const value = Object.create(null) as { message: string }; + value.message = 'something else'; + + expect(() => handleUncaughtException(value, deps)).not.toThrow(); + expect(exitFn).toHaveBeenCalledTimes(1); + }); + + it('does not throw and exits for an unstringifiable non-Error value', () => { + const { deps, exitFn } = createDeps(); + + expect(() => handleUncaughtException(createUnstringifiable(), deps)).not.toThrow(); + expect(exitFn).toHaveBeenCalledTimes(1); + }); +}); + +describe('handleUnhandledRejection', () => { + it('logs a non-Error rejection with its value, distinguishable from an exception', () => { + const { deps, errorFn, exitFn } = createDeps(); + + handleUnhandledRejection('plain failure', deps); + + const logged = errorFn.mock.calls[0][1] as Error; + expect(logged).toBeInstanceOf(Error); + expect(logged.message).toBe('Non-Error rejection: plain failure'); + expect(exitFn).toHaveBeenCalledTimes(1); + }); + + it('keeps the process alive for a tolerated Spark constructor name', () => { + const { deps, errorFn, exitFn } = createDeps(); + const error = new SparkNetworkError('network blip'); + + handleUnhandledRejection(error, deps); + + expect(exitFn).not.toHaveBeenCalled(); + expect(errorFn).toHaveBeenCalledTimes(1); + expect(errorFn).toHaveBeenCalledWith('Spark SDK unhandled rejection (process kept alive):', error); + }); + + it('keeps the process alive for a tolerated Channel has been shut down message', () => { + const { deps, exitFn } = createDeps(); + const error = new Error('Channel has been shut down'); + + handleUnhandledRejection(error, deps); + + expect(exitFn).not.toHaveBeenCalled(); + }); + + it('exits once for a non-tolerated error', () => { + const { deps, errorFn, exitFn } = createDeps(); + const error = new Error('boom'); + + handleUnhandledRejection(error, deps); + + expect(exitFn).toHaveBeenCalledTimes(1); + expect(errorFn).toHaveBeenCalledTimes(1); + expect(errorFn).toHaveBeenCalledWith('Unhandled rejection, shutting down:', error); + }); + + it('does not throw on a hostile Proxy and still exits when not tolerated', () => { + const { deps, exitFn } = createDeps(); + const hostile = createHostileProxy(); + + expect(() => handleUnhandledRejection(hostile, deps)).not.toThrow(); + expect(exitFn).toHaveBeenCalledTimes(1); + }); + + it('does not throw when logger.error always throws and still exits for a non-tolerated error', () => { + const errorFn = jest.fn(() => { + throw new Error('logger is broken'); + }); + const { deps, exitFn } = createDeps({ errorFn }); + + expect(() => handleUnhandledRejection(new Error('boom'), deps)).not.toThrow(); + expect(exitFn).toHaveBeenCalledTimes(1); + }); + + it('does not throw when logger.error always throws for a tolerated error and does not exit', () => { + const errorFn = jest.fn(() => { + throw new Error('logger is broken'); + }); + const { deps, exitFn } = createDeps({ errorFn }); + + expect(() => handleUnhandledRejection(new SparkNetworkError('network blip'), deps)).not.toThrow(); + expect(exitFn).not.toHaveBeenCalled(); + }); + + it('does not throw and exits for undefined', () => { + const { deps, exitFn } = createDeps(); + + expect(() => handleUnhandledRejection(undefined, deps)).not.toThrow(); + expect(exitFn).toHaveBeenCalledTimes(1); + }); + + it('does not throw and exits for null', () => { + const { deps, exitFn } = createDeps(); + + expect(() => handleUnhandledRejection(null, deps)).not.toThrow(); + expect(exitFn).toHaveBeenCalledTimes(1); + }); + + it('does not throw and exits for a plain string', () => { + const { deps, exitFn } = createDeps(); + + expect(() => handleUnhandledRejection('plain failure', deps)).not.toThrow(); + expect(exitFn).toHaveBeenCalledTimes(1); + }); + + it('does not throw and exits for a number', () => { + const { deps, exitFn } = createDeps(); + + expect(() => handleUnhandledRejection(42, deps)).not.toThrow(); + expect(exitFn).toHaveBeenCalledTimes(1); + }); + + it('does not throw and exits for an object without a prototype', () => { + const { deps, exitFn } = createDeps(); + const value = Object.create(null) as Record; + + expect(() => handleUnhandledRejection(value, deps)).not.toThrow(); + expect(exitFn).toHaveBeenCalledTimes(1); + }); + + it('does not throw and exits for a null-prototype object with a non-tolerating message', () => { + const { deps, exitFn } = createDeps(); + const value = Object.create(null) as { message: string }; + value.message = 'something else'; + + expect(() => handleUnhandledRejection(value, deps)).not.toThrow(); + expect(exitFn).toHaveBeenCalledTimes(1); + }); + + it('does not throw and exits for an unstringifiable non-Error value', () => { + const { deps, exitFn } = createDeps(); + + expect(() => handleUnhandledRejection(createUnstringifiable(), deps)).not.toThrow(); + expect(exitFn).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/shared/utils/__tests__/process-error-policy.spec.ts b/src/shared/utils/__tests__/process-error-policy.spec.ts new file mode 100644 index 0000000000..816f05d3d2 --- /dev/null +++ b/src/shared/utils/__tests__/process-error-policy.spec.ts @@ -0,0 +1,67 @@ +import { isToleratedProcessError } from '../process-error-policy'; + +describe('isToleratedProcessError', () => { + it('returns false for undefined', () => { + expect(isToleratedProcessError(undefined)).toBe(false); + }); + + it('returns false for null', () => { + expect(isToleratedProcessError(null)).toBe(false); + }); + + it('returns false for a plain string', () => { + expect(isToleratedProcessError('Channel has been shut down')).toBe(false); + }); + + it('returns false for a number', () => { + expect(isToleratedProcessError(42)).toBe(false); + }); + + it('returns false for an object without a message property', () => { + expect(isToleratedProcessError({ code: 'ECONNRESET' })).toBe(false); + }); + + it('returns false for an object created with a null prototype', () => { + expect(isToleratedProcessError(Object.create(null))).toBe(false); + }); + + it('returns false for a non-matching Error', () => { + expect(isToleratedProcessError(new Error('something else failed'))).toBe(false); + }); + + it('returns true for an Error whose message contains Channel has been shut down', () => { + expect(isToleratedProcessError(new Error('Channel has been shut down'))).toBe(true); + }); + + it('returns true for an Error whose constructor name contains Spark', () => { + class SparkNetworkError extends Error {} + expect(isToleratedProcessError(new SparkNetworkError('network blip'))).toBe(true); + }); + + it('returns true for a plain object with a matching message', () => { + expect(isToleratedProcessError({ message: 'grpc: Channel has been shut down' })).toBe(true); + }); + + it('returns true for a null-prototype object with a matching message', () => { + const error = Object.create(null) as { message: string }; + error.message = 'Channel has been shut down'; + expect(isToleratedProcessError(error)).toBe(true); + }); + + it('returns false when message is present but not a string', () => { + expect(isToleratedProcessError({ message: 123 })).toBe(false); + }); + + it('returns false when property access throws', () => { + const error = new Proxy( + {}, + { + get() { + throw new Error('hostile getter'); + }, + }, + ); + + expect(isToleratedProcessError(error)).toBe(false); + }); +}); diff --git a/src/shared/utils/__tests__/safe-log.spec.ts b/src/shared/utils/__tests__/safe-log.spec.ts new file mode 100644 index 0000000000..92de539e1a --- /dev/null +++ b/src/shared/utils/__tests__/safe-log.spec.ts @@ -0,0 +1,42 @@ +import { DfxLogger } from '../../services/dfx-logger'; +import { safeLogError } from '../safe-log'; + +describe('safeLogError', () => { + it('forwards message and error to logger.error without throwing', () => { + const errorFn = jest.fn(); + const logger = { error: errorFn } as unknown as DfxLogger; + const error = new Error('boom'); + + expect(() => safeLogError(logger, 'something failed:', error)).not.toThrow(); + expect(errorFn).toHaveBeenCalledTimes(1); + expect(errorFn).toHaveBeenCalledWith('something failed:', error); + }); + + it('falls back to a message-only log when logger.error throws and does not rethrow', () => { + const errorFn = jest + .fn() + .mockImplementationOnce(() => { + // Simulate DfxLogger.format / span.recordException hitting a hostile Proxy getter. + throw new Error('hostile stack getter'); + }) + .mockImplementationOnce(() => undefined); + const logger = { error: errorFn } as unknown as DfxLogger; + const error = new Error('boom'); + + expect(() => safeLogError(logger, 'something failed:', error)).not.toThrow(); + expect(errorFn).toHaveBeenCalledTimes(2); + expect(errorFn).toHaveBeenNthCalledWith(1, 'something failed:', error); + expect(errorFn).toHaveBeenNthCalledWith(2, 'something failed: (original error could not be logged)'); + expect(errorFn.mock.calls[1]).toHaveLength(1); + }); + + it('swallows a second failure so a broken logger cannot end the process', () => { + const errorFn = jest.fn().mockImplementation(() => { + throw new Error('logger is broken'); + }); + const logger = { error: errorFn } as unknown as DfxLogger; + + expect(() => safeLogError(logger, 'something failed:', new Error('boom'))).not.toThrow(); + expect(errorFn).toHaveBeenCalledTimes(2); + }); +}); diff --git a/src/shared/utils/process-error-handlers.ts b/src/shared/utils/process-error-handlers.ts new file mode 100644 index 0000000000..f577cb9b00 --- /dev/null +++ b/src/shared/utils/process-error-handlers.ts @@ -0,0 +1,68 @@ +import { DfxLogger } from '../services/dfx-logger'; +import { isToleratedProcessError } from './process-error-policy'; +import { safeLogError } from './safe-log'; + +export interface ProcessErrorHandlerDeps { + logger: DfxLogger; + exit: () => void; +} + +/** Decides whether an uncaught exception keeps the process alive, and logs it either way. */ +export function handleUncaughtException(error: unknown, deps: ProcessErrorHandlerDeps): void { + // `throw 'x'` is legal, so an uncaught exception is not necessarily an Error either. Normalize + // for the logger and test the policy against the original value, exactly as for rejections. + const loggable = toLoggableError(error, 'exception'); + + if (isToleratedProcessError(error)) { + safeLogError(deps.logger, 'Spark SDK uncaught exception (process kept alive):', loggable); + return; + } + + // exit() must run even if logging throws; safeLogError already swallows logger failures, + // but try/finally keeps the shutdown decision independent of the log path. + try { + safeLogError(deps.logger, 'Uncaught exception, shutting down:', loggable); + } finally { + deps.exit(); + } +} + +/** Same decision for an unhandled promise rejection, whose reason can be any value at all. */ +export function handleUnhandledRejection(reason: unknown, deps: ProcessErrorHandlerDeps): void { + // A rejection can carry any value, not just an Error. Normalize for the logger, but test the + // policy against the original value - isToleratedProcessError inspects the constructor name. + // + // `instanceof` is guarded because it is not safe on an arbitrary value either: it consults the + // prototype chain, and a Proxy with a throwing getPrototypeOf trap makes the expression itself + // throw. Unguarded, that throw happens before the policy is ever consulted, and a rejection the + // policy would have tolerated ends up killing the process anyway. + const error = toLoggableError(reason, 'rejection'); + + if (isToleratedProcessError(reason)) { + safeLogError(deps.logger, 'Spark SDK unhandled rejection (process kept alive):', error); + return; + } + + try { + safeLogError(deps.logger, 'Unhandled rejection, shutting down:', error); + } finally { + deps.exit(); + } +} + +function safeStringify(value: unknown): string { + try { + return String(value); + } catch { + return ''; + } +} + +function toLoggableError(reason: unknown, kind: 'exception' | 'rejection'): Error { + try { + if (reason instanceof Error) return reason; + } catch { + // A hostile prototype trap: fall through and wrap it like any other non-Error value. + } + return new Error(`Non-Error ${kind}: ${safeStringify(reason)}`); +} diff --git a/src/shared/utils/process-error-policy.ts b/src/shared/utils/process-error-policy.ts new file mode 100644 index 0000000000..954aca6729 --- /dev/null +++ b/src/shared/utils/process-error-policy.ts @@ -0,0 +1,44 @@ +/** + * Errors the process deliberately survives instead of exiting on. + * + * The Spark SDK can raise process-level failures whose constructor name contains + * "Spark" or whose message contains "Channel has been shut down". SparkClient.call() + * already treats the latter as an expected operating condition and reinitializes the + * wallet when it sees it. Applying the same policy at the process boundary keeps the + * process alive when the same failure escapes outside call() (for example during + * client boot without network connectivity) as either an uncaught exception or an + * unhandled promise rejection. Unrelated process errors are not tolerated. + */ +export function isToleratedProcessError(error: unknown): boolean { + try { + const constructorName = getConstructorName(error); + if (constructorName.includes('Spark')) { + return true; + } + + const message = getMessage(error); + return message.includes('Channel has been shut down'); + } catch { + // Rejection/exception reasons can be arbitrary values, including objects with + // throwing getters. Never let the policy check itself crash the process. + return false; + } +} + +function getConstructorName(error: unknown): string { + if (error === null || error === undefined) { + return ''; + } + + const name = (error as { constructor?: { name?: unknown } }).constructor?.name; + return typeof name === 'string' ? name : ''; +} + +function getMessage(error: unknown): string { + if (error === null || error === undefined) { + return ''; + } + + const message = (error as { message?: unknown }).message; + return typeof message === 'string' ? message : ''; +} diff --git a/src/shared/utils/safe-log.ts b/src/shared/utils/safe-log.ts new file mode 100644 index 0000000000..11e3d84d22 --- /dev/null +++ b/src/shared/utils/safe-log.ts @@ -0,0 +1,20 @@ +import { DfxLogger } from '../services/dfx-logger'; + +/** + * Wraps logger.error(message, error) in try/catch. On failure (e.g. a hostile Proxy + * with a throwing getter on stack/message/name), falls back to logging only the + * message string, without touching any property of the original error object again. + */ +export function safeLogError(logger: DfxLogger, message: string, error: Error): void { + try { + logger.error(message, error); + } catch { + try { + // Do not pass `error` again: any property access on a hostile Proxy may throw. + logger.error(`${message} (original error could not be logged)`); + } catch { + // The logger itself is broken. Losing the line is bad; throwing out of a process-level + // handler that has already decided to keep the process alive would be worse. + } + } +}