From f50e90fb1dd52e760e621019b7b03a33306c5254 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Fri, 7 Aug 2026 21:17:49 +0000 Subject: [PATCH 01/19] Start full-stack E2E integration branch From 3fc7cdb2c081fd564fa34c73b0a5ae68242b1f59 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Fri, 7 Aug 2026 21:35:15 +0000 Subject: [PATCH 02/19] Add full-stack E2E CI workflow --- .github/workflows/e2e-stack.yml | 93 +++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) create mode 100644 .github/workflows/e2e-stack.yml diff --git a/.github/workflows/e2e-stack.yml b/.github/workflows/e2e-stack.yml new file mode 100644 index 0000000000..8e92891187 --- /dev/null +++ b/.github/workflows/e2e-stack.yml @@ -0,0 +1,93 @@ +# 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: + services_ref: + description: Ref of DFXswiss/services to check out for the e2e-stack harness + required: false + default: develop + 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 + + - name: Set up Node.js 20 + uses: actions/setup-node@v4 + with: + node-version: '20' + cache: 'npm' + cache-dependency-path: services-repo/package-lock.json + + - name: Build API image from PR head + run: docker build -t dfx-api:e2e --build-arg GIT_COMMIT=${{ github.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 + + - name: Run Playwright tests + run: | + docker compose -p dfx-e2e-stack \ + -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@v4 + 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 From e431b155632b569df17238cc5367918c9091b100 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Fri, 7 Aug 2026 22:13:40 +0000 Subject: [PATCH 03/19] Tolerate Spark SDK unhandled rejections like uncaught exceptions The uncaughtException handler already survives Spark SDK failures identified by "Channel has been shut down", because SparkClient.call() treats that as an expected operating condition and reinitializes the wallet on it. The same failure also surfaces as an unhandled promise rejection (e.g. SparkClient boot without network connectivity), which had no matching handler and terminated the process. Extract the toleration rule into isToleratedProcessError() and apply it to both channels. --- src/main.ts | 30 +++++++-- .../__tests__/process-error-policy.spec.ts | 67 +++++++++++++++++++ src/shared/utils/process-error-policy.ts | 44 ++++++++++++ 3 files changed, 137 insertions(+), 4 deletions(-) create mode 100644 src/shared/utils/__tests__/process-error-policy.spec.ts create mode 100644 src/shared/utils/process-error-policy.ts diff --git a/src/main.ts b/src/main.ts index f4c4b1a5b4..e227f37e09 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 { isToleratedProcessError } from './shared/utils/process-error-policy'; import { AppModule } from './app.module'; import { Config, Environment } from './config/config'; import { ApiExceptionFilter } from './shared/filters/exception.filter'; @@ -35,10 +36,7 @@ import { PricingService } from './subdomains/supporting/pricing/services/pricing 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) { + if (isToleratedProcessError(error)) { logger.error('Spark SDK uncaught exception (process kept alive):', error); return; } @@ -47,6 +45,30 @@ process.on('uncaughtException', (error) => { process.exit(1); }); +function safeStringify(value: unknown): string { + try { + return String(value); + } catch { + return ''; + } +} + +process.on('unhandledRejection', (reason) => { + const logger = new DfxLogger('UnhandledRejection'); + + // 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. + const error = reason instanceof Error ? reason : new Error(`Non-Error rejection: ${safeStringify(reason)}`); + + if (isToleratedProcessError(reason)) { + logger.error('Spark SDK unhandled rejection (process kept alive):', error); + return; + } + + logger.error('Unhandled rejection, shutting down:', error); + process.exit(1); +}); + async function bootstrap() { // Observability is initialized in src/tracing.ts (imported above): the // OpenTelemetry SDK auto-instruments HTTP/DB/NestJS and exports traces via 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/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 : ''; +} From f33ea922ee7a6c662f56e70d6ecf5468bad74947 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Fri, 7 Aug 2026 22:22:18 +0000 Subject: [PATCH 04/19] Catch fire-and-forget L2 bridge init failures instead of crashing the process PolygonClient and ArbitrumClient both kick off their L2 network initialization in the constructor as a fire-and-forget call with no error handling. A rejection there becomes an unhandled promise rejection, which the process now correctly treats as fatal since the previous commit added a matching unhandledRejection handler - exposing that these two initializations could take the whole process down on a failure that should only affect the L2 bridge (e.g. an unsupported network/version combination, or a temporarily unreachable provider). Log and swallow the error instead, since these initializations are meant to run concurrently in the background. --- src/integration/blockchain/arbitrum/arbitrum-client.ts | 2 +- src/integration/blockchain/polygon/polygon-client.ts | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/integration/blockchain/arbitrum/arbitrum-client.ts b/src/integration/blockchain/arbitrum/arbitrum-client.ts index ac499922b5..fc0fc779f3 100644 --- a/src/integration/blockchain/arbitrum/arbitrum-client.ts +++ b/src/integration/blockchain/arbitrum/arbitrum-client.ts @@ -36,7 +36,7 @@ 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(); + void this.initL2Network().catch((e) => this.logger.error('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..db877f6d85 100644 --- a/src/integration/blockchain/polygon/polygon-client.ts +++ b/src/integration/blockchain/polygon/polygon-client.ts @@ -33,7 +33,9 @@ export class PolygonClient extends EvmClient implements L2BridgeEvmClient { const { polygonWalletAddress } = GetConfig().blockchain.polygon; this.posClient = new POSClient(); - void this.initPolygonNetwork(ethWalletAddress, polygonWalletAddress); + void this.initPolygonNetwork(ethWalletAddress, polygonWalletAddress).catch((e) => + this.logger.error('Polygon L2 network initialization failed:', e), + ); this.l2TxIdCache = new Set(); } From ba4d8f6dff2d763b0cfb9baa6b7aac3438beb22c Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Sat, 8 Aug 2026 01:39:53 +0000 Subject: [PATCH 05/19] Say why the harness is missing instead of failing on a missing path --- .github/workflows/e2e-stack.yml | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/.github/workflows/e2e-stack.yml b/.github/workflows/e2e-stack.yml index 8e92891187..6d88769515 100644 --- a/.github/workflows/e2e-stack.yml +++ b/.github/workflows/e2e-stack.yml @@ -45,6 +45,19 @@ jobs: ref: ${{ inputs.services_ref || 'develop' }} path: services-repo + # Fails with a sentence someone can act on instead of `up.sh: No such file or directory`. + # The harness lives in the frontend repository, so this job cannot pass until the pull + # request that adds it has merged there. That ordering is deliberate: a red check here is + # the reminder, and it clears itself once the other side lands. + - name: Check that the harness is present + run: | + if [ ! -x services-repo/e2e-stack/scripts/up.sh ]; then + echo "::error::The e2e-stack harness is not present in DFXswiss/services@${{ inputs.services_ref || 'develop' }}." + echo "::error::Merge the frontend pull request that adds e2e-stack/ first. To try this job" + echo "::error::against a branch beforehand, dispatch it manually with services_ref set." + exit 1 + fi + - name: Set up Node.js 20 uses: actions/setup-node@v4 with: From 4380e118a1a156b1a1e54befc46e55acf97fc098 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Sat, 8 Aug 2026 04:44:11 +0000 Subject: [PATCH 06/19] Drop a Node setup step this job never uses --- .github/workflows/e2e-stack.yml | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/.github/workflows/e2e-stack.yml b/.github/workflows/e2e-stack.yml index 6d88769515..3f66314adf 100644 --- a/.github/workflows/e2e-stack.yml +++ b/.github/workflows/e2e-stack.yml @@ -58,12 +58,9 @@ jobs: exit 1 fi - - name: Set up Node.js 20 - uses: actions/setup-node@v4 - with: - node-version: '20' - cache: 'npm' - cache-dependency-path: services-repo/package-lock.json + # 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 run: docker build -t dfx-api:e2e --build-arg GIT_COMMIT=${{ github.sha }} api-repo From e63728160afecbdd23dd7ce997f6be014193236a Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Sat, 8 Aug 2026 05:25:05 +0000 Subject: [PATCH 07/19] Guard the instanceof check that runs before the tolerance policy --- src/main.ts | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/src/main.ts b/src/main.ts index e227f37e09..29b1c6029f 100644 --- a/src/main.ts +++ b/src/main.ts @@ -53,12 +53,26 @@ function safeStringify(value: unknown): string { } } +function toLoggableError(reason: unknown): 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 rejection: ${safeStringify(reason)}`); +} + process.on('unhandledRejection', (reason) => { const logger = new DfxLogger('UnhandledRejection'); // 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. - const error = reason instanceof Error ? reason : new Error(`Non-Error rejection: ${safeStringify(reason)}`); + // + // `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); if (isToleratedProcessError(reason)) { logger.error('Spark SDK unhandled rejection (process kept alive):', error); From d4324fe03dd3131bceff427e8fee5e3f556e1d52 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Sat, 8 Aug 2026 07:44:35 +0000 Subject: [PATCH 08/19] Bootstrap the e2e harness from the pull request that introduces it The harness lives in the companion frontend repository and is not on its default branch yet, so this job failed on ordering rather than on anything about the API. It now falls back to the pull request head that carries the harness, warns while it does so, and disables itself the moment the harness reaches the default branch. An explicitly supplied services_ref keeps failing hard when the harness is not on it: that is a caller mistake, not a bootstrap case. --- .github/workflows/e2e-stack.yml | 59 +++++++++++++++++++++++++++------ 1 file changed, 48 insertions(+), 11 deletions(-) diff --git a/.github/workflows/e2e-stack.yml b/.github/workflows/e2e-stack.yml index 3f66314adf..2cc2cf3215 100644 --- a/.github/workflows/e2e-stack.yml +++ b/.github/workflows/e2e-stack.yml @@ -14,10 +14,14 @@ on: - 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 to check out for the e2e-stack harness + description: Ref of DFXswiss/services holding the e2e-stack harness (empty = develop) required: false - default: develop type: string permissions: @@ -45,16 +49,47 @@ jobs: ref: ${{ inputs.services_ref || 'develop' }} path: services-repo - # Fails with a sentence someone can act on instead of `up.sh: No such file or directory`. - # The harness lives in the frontend repository, so this job cannot pass until the pull - # request that adds it has merged there. That ordering is deliberate: a red check here is - # the reminder, and it clears itself once the other side lands. - - name: Check that the harness is present + # 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 }} + run: | + 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 + 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 "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. refs/pull/1288/head stays valid after merge (GitHub + # keeps PR refs), so this is not a dead branch name. + - 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 not present in DFXswiss/services@${{ inputs.services_ref || 'develop' }}." - echo "::error::Merge the frontend pull request that adds e2e-stack/ first. To try this job" - echo "::error::against a branch beforehand, dispatch it manually with services_ref set." + 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 @@ -63,7 +98,9 @@ jobs: # npx on the runner itself. - name: Build API image from PR head - run: docker build -t dfx-api:e2e --build-arg GIT_COMMIT=${{ github.sha }} api-repo + 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 From 2ffca3697af6fb38b0022c4f8255eda5e9a83a97 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Sat, 8 Aug 2026 07:58:29 +0000 Subject: [PATCH 09/19] Say why the Arbitrum catch is there initL2Network() handles its own failures today, so the catch never fires. Without a note, the next reader has to re-derive that and may well delete it. --- src/integration/blockchain/arbitrum/arbitrum-client.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/integration/blockchain/arbitrum/arbitrum-client.ts b/src/integration/blockchain/arbitrum/arbitrum-client.ts index fc0fc779f3..fde932dc18 100644 --- a/src/integration/blockchain/arbitrum/arbitrum-client.ts +++ b/src/integration/blockchain/arbitrum/arbitrum-client.ts @@ -36,6 +36,9 @@ export class ArbitrumClient extends EvmClient implements L2BridgeEvmClient { this.l1Provider = new ethers.providers.JsonRpcProvider(ethereumGateway); this.l1Wallet = new ethers.Wallet(ethWalletPrivateKey, this.l1Provider); + // 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. void this.initL2Network().catch((e) => this.logger.error('Arbitrum L2 network initialization failed:', e)); } From 868e9637e568abc1503351ed336be1e51ca9ad7f Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Sat, 8 Aug 2026 07:58:29 +0000 Subject: [PATCH 10/19] Let the harness bootstrap expire with the pull request it points at refs/pull/1288/head stays fetchable forever, so a later rename of the harness scripts on the default branch would have quietly re-armed the fallback and tested against a years-old frontend. The step now refuses to bootstrap once that pull request is no longer open, and says which block to delete. --- .github/workflows/e2e-stack.yml | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/.github/workflows/e2e-stack.yml b/.github/workflows/e2e-stack.yml index 2cc2cf3215..021a0c2ebc 100644 --- a/.github/workflows/e2e-stack.yml +++ b/.github/workflows/e2e-stack.yml @@ -57,7 +57,9 @@ jobs: 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 @@ -66,16 +68,34 @@ jobs: echo "::error::ref that contains e2e-stack/, or omit services_ref to use develop (with fallback)." exit 1 else + if ! state="$(gh api repos/DFXswiss/services/pulls/1288 --jq .state 2>&1)"; then + echo "::error::Could not determine the state of DFXswiss/services#1288 (gh api call failed):" + echo "::error::${state}" + exit 1 + fi + 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. refs/pull/1288/head stays valid after merge (GitHub - # keeps PR refs), so this is not a dead branch name. + # this step is skipped. After services#1288 is merged or closed, Resolve harness + # location fails loud instead of bootstrapping, so this step never runs against + # a spent PR ref (refs/pull/1288/head 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 From 2968d0817414e11d655bcdc75307600d7904c9eb Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Sat, 8 Aug 2026 08:20:31 +0000 Subject: [PATCH 11/19] Keep the logging path from undoing the tolerance decision MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Once a rejection is judged tolerable, the process must survive it. The log call that follows reads properties off the rejection value, so a hostile getter could throw there and take the process down anyway — after the policy had already decided to keep it. Logging now falls back to the message alone. Also from the same review: read gh's answer without its stderr mixed in, so a warning on an open pull request cannot make it look closed. --- .github/workflows/e2e-stack.yml | 19 ++++++++---- src/main.ts | 9 +++--- src/shared/utils/__tests__/safe-log.spec.ts | 32 +++++++++++++++++++++ src/shared/utils/safe-log.ts | 15 ++++++++++ 4 files changed, 66 insertions(+), 9 deletions(-) create mode 100644 src/shared/utils/__tests__/safe-log.spec.ts create mode 100644 src/shared/utils/safe-log.ts diff --git a/.github/workflows/e2e-stack.yml b/.github/workflows/e2e-stack.yml index 021a0c2ebc..349c148c22 100644 --- a/.github/workflows/e2e-stack.yml +++ b/.github/workflows/e2e-stack.yml @@ -68,11 +68,16 @@ jobs: echo "::error::ref that contains e2e-stack/, or omit services_ref to use develop (with fallback)." exit 1 else - if ! state="$(gh api repos/DFXswiss/services/pulls/1288 --jq .state 2>&1)"; then + # 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::${state}" + 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 @@ -93,9 +98,13 @@ jobs: # 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, Resolve harness - # location fails loud instead of bootstrapping, so this step never runs against - # a spent PR ref (refs/pull/1288/head stays fetchable on GitHub forever). + # 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 diff --git a/src/main.ts b/src/main.ts index 29b1c6029f..5f28fd2236 100644 --- a/src/main.ts +++ b/src/main.ts @@ -18,6 +18,7 @@ import morgan from 'morgan'; import { join } from 'path'; import { getVerifiedIp } from './shared/utils/ip.util'; import { isToleratedProcessError } from './shared/utils/process-error-policy'; +import { safeLogError } from './shared/utils/safe-log'; import { AppModule } from './app.module'; import { Config, Environment } from './config/config'; import { ApiExceptionFilter } from './shared/filters/exception.filter'; @@ -37,11 +38,11 @@ process.on('uncaughtException', (error) => { const logger = new DfxLogger('UncaughtException'); if (isToleratedProcessError(error)) { - logger.error('Spark SDK uncaught exception (process kept alive):', error); + safeLogError(logger, 'Spark SDK uncaught exception (process kept alive):', error); return; } - logger.error('Uncaught exception, shutting down:', error); + safeLogError(logger, 'Uncaught exception, shutting down:', error); process.exit(1); }); @@ -75,11 +76,11 @@ process.on('unhandledRejection', (reason) => { const error = toLoggableError(reason); if (isToleratedProcessError(reason)) { - logger.error('Spark SDK unhandled rejection (process kept alive):', error); + safeLogError(logger, 'Spark SDK unhandled rejection (process kept alive):', error); return; } - logger.error('Unhandled rejection, shutting down:', error); + safeLogError(logger, 'Unhandled rejection, shutting down:', error); process.exit(1); }); 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..1004288425 --- /dev/null +++ b/src/shared/utils/__tests__/safe-log.spec.ts @@ -0,0 +1,32 @@ +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); + }); +}); diff --git a/src/shared/utils/safe-log.ts b/src/shared/utils/safe-log.ts new file mode 100644 index 0000000000..7acf884af6 --- /dev/null +++ b/src/shared/utils/safe-log.ts @@ -0,0 +1,15 @@ +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 { + // Do not pass `error` again: any property access on a hostile Proxy may throw. + logger.error(`${message} (original error could not be logged)`); + } +} From d2aab679f84dc68211448b42f77b6acb5825ea34 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Sat, 8 Aug 2026 09:03:22 +0000 Subject: [PATCH 12/19] Let a broken logger cost the line, not the process The fallback log call was itself unguarded, so a logger that fails for its own reasons could still throw out of a handler that had already decided to keep the process alive. --- src/shared/utils/__tests__/safe-log.spec.ts | 10 ++++++++++ src/shared/utils/safe-log.ts | 9 +++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/shared/utils/__tests__/safe-log.spec.ts b/src/shared/utils/__tests__/safe-log.spec.ts index 1004288425..92de539e1a 100644 --- a/src/shared/utils/__tests__/safe-log.spec.ts +++ b/src/shared/utils/__tests__/safe-log.spec.ts @@ -29,4 +29,14 @@ describe('safeLogError', () => { 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/safe-log.ts b/src/shared/utils/safe-log.ts index 7acf884af6..11e3d84d22 100644 --- a/src/shared/utils/safe-log.ts +++ b/src/shared/utils/safe-log.ts @@ -9,7 +9,12 @@ export function safeLogError(logger: DfxLogger, message: string, error: Error): try { logger.error(message, error); } catch { - // Do not pass `error` again: any property access on a hostile Proxy may throw. - logger.error(`${message} (original error could not be logged)`); + 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. + } } } From 115420b9673b1687166f1b96ecc2e88a13dfb5dc Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Sat, 8 Aug 2026 09:09:27 +0000 Subject: [PATCH 13/19] Pass the harness env file to the test run up.sh records the values it resolved there; a compose run that resolves them differently recreates the API container in the middle of the suite. --- .github/workflows/e2e-stack.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/e2e-stack.yml b/.github/workflows/e2e-stack.yml index 349c148c22..bb3c3d3d8d 100644 --- a/.github/workflows/e2e-stack.yml +++ b/.github/workflows/e2e-stack.yml @@ -139,9 +139,12 @@ jobs: - 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 run: | docker compose -p dfx-e2e-stack \ + --env-file services-repo/e2e-stack/.env \ -f services-repo/e2e-stack/compose.yml \ -f services-repo/e2e-stack/compose.tests.yml \ run --name dfx-e2e-stack-tests tests From f8d37c000ee3a2d19170bddd915efacc71d0bd9c Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Sat, 8 Aug 2026 09:13:33 +0000 Subject: [PATCH 14/19] Make the process error handlers testable, and test them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The handlers decide whether the process survives, and nothing covered that decision: they sat in the module body of main.ts, where reaching them means starting the whole application. They now live behind two functions that take the logger and the exit action, so the guarantee itself is asserted — tolerated errors leave the process alone, everything else exits, a hostile rejection value throws nowhere, and a logger that always throws changes neither outcome. --- src/main.ts | 58 +--- .../__tests__/process-error-handlers.spec.ts | 269 ++++++++++++++++++ src/shared/utils/process-error-handlers.ts | 68 +++++ 3 files changed, 344 insertions(+), 51 deletions(-) create mode 100644 src/shared/utils/__tests__/process-error-handlers.spec.ts create mode 100644 src/shared/utils/process-error-handlers.ts diff --git a/src/main.ts b/src/main.ts index 5f28fd2236..e2dba262a0 100644 --- a/src/main.ts +++ b/src/main.ts @@ -17,8 +17,7 @@ import helmet from 'helmet'; import morgan from 'morgan'; import { join } from 'path'; import { getVerifiedIp } from './shared/utils/ip.util'; -import { isToleratedProcessError } from './shared/utils/process-error-policy'; -import { safeLogError } from './shared/utils/safe-log'; +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'; @@ -34,55 +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'); - - if (isToleratedProcessError(error)) { - safeLogError(logger, 'Spark SDK uncaught exception (process kept alive):', error); - return; - } - - safeLogError(logger, 'Uncaught exception, shutting down:', error); - process.exit(1); -}); - -function safeStringify(value: unknown): string { - try { - return String(value); - } catch { - return ''; - } -} - -function toLoggableError(reason: unknown): 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 rejection: ${safeStringify(reason)}`); -} - -process.on('unhandledRejection', (reason) => { - const logger = new DfxLogger('UnhandledRejection'); - - // 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); - - if (isToleratedProcessError(reason)) { - safeLogError(logger, 'Spark SDK unhandled rejection (process kept alive):', error); - return; - } - - safeLogError(logger, 'Unhandled rejection, 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..cc2e42723b --- /dev/null +++ b/src/shared/utils/__tests__/process-error-handlers.spec.ts @@ -0,0 +1,269 @@ +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('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('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/process-error-handlers.ts b/src/shared/utils/process-error-handlers.ts new file mode 100644 index 0000000000..aff03135e2 --- /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 { + // Node's uncaughtException always delivers an Error; the parameter is typed as unknown so + // tests can exercise hostile and non-Error values without starting the Nest app. + const loggable = error as Error; + + 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); + + 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): 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 rejection: ${safeStringify(reason)}`); +} From 9040bfbd1f890a24479ded69661192745fe24166 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Sat, 8 Aug 2026 12:57:35 +0000 Subject: [PATCH 15/19] Follow the harness's env file to its new name The companion repository renamed the file its stack script generates, so that it stops overwriting the one a developer keeps. --- .github/workflows/e2e-stack.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/e2e-stack.yml b/.github/workflows/e2e-stack.yml index bb3c3d3d8d..34f1bab127 100644 --- a/.github/workflows/e2e-stack.yml +++ b/.github/workflows/e2e-stack.yml @@ -144,7 +144,7 @@ jobs: - name: Run Playwright tests run: | docker compose -p dfx-e2e-stack \ - --env-file services-repo/e2e-stack/.env \ + --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 From 69b55b1242781886ca05dd08fe1836e18ae1f150 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Sat, 8 Aug 2026 13:52:12 +0000 Subject: [PATCH 16/19] Normalize a non-Error uncaught exception too The rejection path already wrapped values that are not Errors before logging them; the exception path cast instead, on the assumption that Node only ever delivers an Error. is legal, and the assumption cost the log line its content. Both paths now say which of the two they are reporting, and the tests read what was logged rather than only that nothing threw. --- .../__tests__/process-error-handlers.spec.ts | 22 +++++++++++++++++++ src/shared/utils/process-error-handlers.ts | 12 +++++----- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/src/shared/utils/__tests__/process-error-handlers.spec.ts b/src/shared/utils/__tests__/process-error-handlers.spec.ts index cc2e42723b..dc80fa7000 100644 --- a/src/shared/utils/__tests__/process-error-handlers.spec.ts +++ b/src/shared/utils/__tests__/process-error-handlers.spec.ts @@ -123,6 +123,28 @@ describe('handleUncaughtException', () => { expect(exitFn).toHaveBeenCalledTimes(1); }); + 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('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('does not throw and exits for a number', () => { const { deps, exitFn } = createDeps(); diff --git a/src/shared/utils/process-error-handlers.ts b/src/shared/utils/process-error-handlers.ts index aff03135e2..ff40f725b0 100644 --- a/src/shared/utils/process-error-handlers.ts +++ b/src/shared/utils/process-error-handlers.ts @@ -9,9 +9,9 @@ export interface ProcessErrorHandlerDeps { /** Decides whether an uncaught exception keeps the process alive, and logs it either way. */ export function handleUncaughtException(error: unknown, deps: ProcessErrorHandlerDeps): void { - // Node's uncaughtException always delivers an Error; the parameter is typed as unknown so - // tests can exercise hostile and non-Error values without starting the Nest app. - const loggable = error as Error; + // `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); @@ -36,7 +36,7 @@ export function handleUnhandledRejection(reason: unknown, deps: ProcessErrorHand // 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); + const error = toLoggableError(reason, 'rejection'); if (isToleratedProcessError(reason)) { safeLogError(deps.logger, 'Spark SDK unhandled rejection (process kept alive):', error); @@ -58,11 +58,11 @@ function safeStringify(value: unknown): string { } } -function toLoggableError(reason: unknown): Error { +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 rejection: ${safeStringify(reason)}`); + return new Error(`Non-Error ${kind}: ${safeStringify(reason)}`); } From 46aef0db5cec7a253c398eda84f34c1c47ac3b91 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Sat, 8 Aug 2026 14:31:10 +0000 Subject: [PATCH 17/19] Stop the fallback text from naming the wrong kind of failure Both handlers share the value-to-text fallback now, so an exception whose value cannot be stringified was reported as an unstringifiable rejection. The wording is neutral, a test reads the message rather than only checking that nothing threw, and the rejection test moved into the block it belongs to. --- .../__tests__/process-error-handlers.spec.ts | 27 ++++++++++++------- src/shared/utils/process-error-handlers.ts | 2 +- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/shared/utils/__tests__/process-error-handlers.spec.ts b/src/shared/utils/__tests__/process-error-handlers.spec.ts index dc80fa7000..3f1c677376 100644 --- a/src/shared/utils/__tests__/process-error-handlers.spec.ts +++ b/src/shared/utils/__tests__/process-error-handlers.spec.ts @@ -123,26 +123,24 @@ describe('handleUncaughtException', () => { expect(exitFn).toHaveBeenCalledTimes(1); }); - it('logs a non-Error rejection with its value, distinguishable from an exception', () => { + it('logs a non-Error exception with its value, distinguishable from a rejection', () => { const { deps, errorFn, exitFn } = createDeps(); - handleUnhandledRejection('plain failure', deps); + handleUncaughtException('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(logged.message).toBe('Non-Error exception: plain failure'); expect(exitFn).toHaveBeenCalledTimes(1); }); - it('logs a non-Error exception with its value, distinguishable from a rejection', () => { - const { deps, errorFn, exitFn } = createDeps(); + it('names the value it could not stringify without calling it a rejection', () => { + const { deps, errorFn } = createDeps(); - handleUncaughtException('plain failure', deps); + handleUncaughtException(createUnstringifiable(), 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); + expect(logged.message).toBe('Non-Error exception: '); }); it('does not throw and exits for a number', () => { @@ -178,6 +176,17 @@ describe('handleUncaughtException', () => { }); 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'); diff --git a/src/shared/utils/process-error-handlers.ts b/src/shared/utils/process-error-handlers.ts index ff40f725b0..f577cb9b00 100644 --- a/src/shared/utils/process-error-handlers.ts +++ b/src/shared/utils/process-error-handlers.ts @@ -54,7 +54,7 @@ function safeStringify(value: unknown): string { try { return String(value); } catch { - return ''; + return ''; } } From 9afe31e0c8287c77c00630b1215bfe20fbc4cda7 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Mon, 10 Aug 2026 08:32:37 +0000 Subject: [PATCH 18/19] Log the bridge init failures the way the rest of this change logs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both new catch callbacks called the logger directly, on a value they did not produce. 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. They go through the same guarded helper as the process handlers now. Also: match the artifact action version the repository already uses elsewhere. --- .github/workflows/e2e-stack.yml | 2 +- src/integration/blockchain/arbitrum/arbitrum-client.ts | 5 ++++- src/integration/blockchain/polygon/polygon-client.ts | 6 +++++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/.github/workflows/e2e-stack.yml b/.github/workflows/e2e-stack.yml index 34f1bab127..c27c6cd451 100644 --- a/.github/workflows/e2e-stack.yml +++ b/.github/workflows/e2e-stack.yml @@ -160,7 +160,7 @@ jobs: - name: Upload e2e report if: always() - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v7 with: name: e2e-stack-report path: e2e-artifacts/ diff --git a/src/integration/blockchain/arbitrum/arbitrum-client.ts b/src/integration/blockchain/arbitrum/arbitrum-client.ts index fde932dc18..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); @@ -39,7 +40,9 @@ export class ArbitrumClient extends EvmClient implements L2BridgeEvmClient { // 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. - void this.initL2Network().catch((e) => this.logger.error('Arbitrum L2 network initialization failed:', e)); + // 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 db877f6d85..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,8 +34,11 @@ export class PolygonClient extends EvmClient implements L2BridgeEvmClient { const { polygonWalletAddress } = GetConfig().blockchain.polygon; this.posClient = new POSClient(); + // 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) => - this.logger.error('Polygon L2 network initialization failed:', e), + safeLogError(this.logger, 'Polygon L2 network initialization failed:', e), ); this.l2TxIdCache = new Set(); From 5495af96af5d1d51b463dd39fda8baf1380120a9 Mon Sep 17 00:00:00 2001 From: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Date: Mon, 10 Aug 2026 10:26:47 +0000 Subject: [PATCH 19/19] Declare the full run so the harness checks route coverage here too The frontend harness only requires every route to have been navigated to when the run says it covered every spec. Without this the job would check ownership alone. --- .github/workflows/e2e-stack.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/e2e-stack.yml b/.github/workflows/e2e-stack.yml index c27c6cd451..4ffde933d5 100644 --- a/.github/workflows/e2e-stack.yml +++ b/.github/workflows/e2e-stack.yml @@ -142,6 +142,9 @@ jobs: # --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 \