diff --git a/.agents/skills/apm-integrations/references/orchestrion.md b/.agents/skills/apm-integrations/references/orchestrion.md index 092303d1cb..0ce4b8e2cd 100644 --- a/.agents/skills/apm-integrations/references/orchestrion.md +++ b/.agents/skills/apm-integrations/references/orchestrion.md @@ -208,6 +208,95 @@ For integrations wrapping multiple methods, create a separate plugin class per m - `ctx.error` — thrown error (on error) - `ctx.currentStore` — set by `startSpan` in `bindStart` +## Propagating Synchronous Errors From `bindStart` + +Subscribers on the prefix `:start` channel and `bindStore` transforms **cannot +propagate a synchronous throw** to the caller of an orchestrion-wrapped +function. Both are wrapped in `try { ... } catch (err) { process.nextTick(() => +triggerUncaughtException(err)); ... }` by Node's `diagnostics_channel` (see +`lib/diagnostics_channel.js`'s `wrapStoreRun` and `publish`). The error +surfaces async as an uncaught exception, **after** the wrapped fn has already +run and the call has returned normally. + +The wrapper's own catch block, however, **does** rethrow: + +```js +return ch.start.runStores(__apm$ctx, () => { + try { + const result = __apm$traced(); + __apm$ctx.result = result; + return result; + } catch (err) { + __apm$ctx.error = err; + ch.error.publish(__apm$ctx); + throw err; // <- propagates the wrapped fn's error to the caller + } finally { + ch.end.publish(__apm$ctx); + } +}); +``` + +When a contract requires `assert.throws(() => wrapped(...))`-style synchronous +propagation from a `:start` observer (the canonical case is AppSec WAF's +`abortController.abort()` model), use the **Proxy-on-arguments pattern**: + +```js +bindStart (ctx) { + // ... normal setup, span creation ... + const abortController = new AbortController() + if (startCh.hasSubscribers) { + startCh.publish({ abortController, args }) // subscribers run sync + if (abortController.signal.aborted) { + // ctx.arguments is the SAME array reference the wrapper spreads into + // the wrapped fn (__apm$wrapped.apply(this, ctx.arguments)). Replace + // arguments[0] with a Proxy whose getters throw AbortError. The + // wrapped fn's first property access (typically a destructure of args) + // triggers the trap → the wrapper's catch+rethrow propagates AbortError + // to the caller. Span lifecycle still completes via end.publish. + ctx.arguments[0] = new Proxy({}, { + get () { throw new AbortError('Aborted') }, + has () { throw new AbortError('Aborted') }, + }) + ctx.ddAborted = true + return ctx.currentStore + } + } + // ... rest of bindStart ... +} + +error (ctx) { + if (ctx.ddAborted) return // abort != error tag + // ... regular error handling ... +} +``` + +Reference implementation: `packages/datadog-plugin-graphql/src/execute.js` +(`apm:graphql:execute:start` contract). + +Why this works: +- `ctx.arguments` and the rewriter's `__apm$arguments` are the same array + reference (confirmed by capturing the rewriter output: `const __apm$traced + = () => __apm$wrapped.apply(this, __apm$arguments)`). +- The wrapped fn's body almost always touches `arguments[0]` on the first + statement (destructure, property read, validation). Any read triggers the + Proxy trap. +- The orchestrion wrapper's `catch { ...; throw err }` propagates the thrown + error synchronously to the caller — confirmed in the rewriter template + (vendor's `code-transformer/index.js`, `wrapSync` function). +- `:end` still fires in `finally`, so the plugin's `end(ctx)` runs as usual + and the span lifecycle completes cleanly. Combined with an `error(ctx)` + that no-ops when `ctx.ddAborted`, the span finishes with `error === 0` + (matches the abort contract). + +When NOT to use this: +- If you control the wrapped fn (e.g., it's a method you can wrap with + shimmer on top of orchestrion), do that — clearer and avoids the + Proxy indirection. +- If the wrapped fn might catch its own errors before they escape (some + user-resolver patterns do this), the AbortError won't propagate. Verify + the wrapped fn does NOT have a top-level `try/catch` around the + property access that triggers the trap. + ## Common Issues ### Wrong filePath diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index cae9183df9..20ea7c6a6e 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -146,6 +146,11 @@ /integration-tests/vitest/ @DataDog/ci-app-libraries /integration-tests/vitest.config.mjs @DataDog/ci-app-libraries /integration-tests/vitest.second-project.config.mjs @DataDog/ci-app-libraries +/integration-tests/vitest.typecheck.config.mjs @DataDog/ci-app-libraries +/integration-tests/tsconfig.typecheck.json @DataDog/ci-app-libraries +/integration-tests/tsconfig.typecheck-fail.json @DataDog/ci-app-libraries +/integration-tests/tsconfig.typecheck-test-management-fail.json @DataDog/ci-app-libraries +/integration-tests/tsconfig.typecheck-test-management-file-fail.json @DataDog/ci-app-libraries /integration-tests/ci-visibility-intake.js @DataDog/ci-app-libraries /integration-tests/ci-visibility-intake.spec.js @DataDog/ci-app-libraries /integration-tests/CODEOWNERS @DataDog/ci-app-libraries diff --git a/.github/actions/coverage/action.yml b/.github/actions/coverage/action.yml index d01ceef7cd..3f481b05b4 100644 --- a/.github/actions/coverage/action.yml +++ b/.github/actions/coverage/action.yml @@ -1,9 +1,13 @@ name: coverage -description: Verify coverage output and upload it to Codecov and Datadog. +description: >- + Verify a job's coverage output and stash it as an artifact. All Green collects every + `coverage-*` artifact, merges them per integration, and uploads the groups to Codecov and + Datadog — so a commit sends ~100 grouped reports instead of one per matrix cell, which kept + Codecov from silently dropping uploads past its per-commit ceiling. inputs: flags: - description: "Codecov flags value" + description: "Coverage flags — the grouping key All Green folds matrix cells by" required: false report-dir: description: "Coverage report directory (defaults to 'coverage')" @@ -12,19 +16,13 @@ inputs: runs: using: composite steps: - # Retry once on failure to work around transient issues (e.g. flaky - # Codecov upload network calls). - - id: attempt - uses: ./.github/actions/coverage/upload - continue-on-error: true - with: - flags: ${{ inputs.flags }} - report-dir: ${{ inputs.report-dir }} - - if: steps.attempt.outcome == 'failure' + - name: Verify coverage output shell: bash - run: sleep 60 - - if: steps.attempt.outcome == 'failure' - uses: ./.github/actions/coverage/upload + run: node scripts/verify-coverage.js --flags "${{ inputs.flags }}" + + - name: Upload coverage artifact + if: always() + uses: ./.github/actions/upload-coverage-artifact with: flags: ${{ inputs.flags }} report-dir: ${{ inputs.report-dir }} diff --git a/.github/actions/coverage/upload/action.yml b/.github/actions/coverage/upload/action.yml deleted file mode 100644 index f74f03241d..0000000000 --- a/.github/actions/coverage/upload/action.yml +++ /dev/null @@ -1,51 +0,0 @@ -name: Coverage Upload -description: Internal implementation; verify and upload coverage. Use `./.github/actions/coverage` instead. - -inputs: - flags: - description: "Codecov flags value" - required: false - report-dir: - description: "Coverage report directory (defaults to 'coverage')" - required: false - default: coverage - -runs: - using: composite - steps: - - name: Verify coverage output - shell: bash - run: node scripts/verify-coverage.js --flags "${{ inputs.flags }}" - - # `master-coverage` is the flag .codecov.yml gates codecov/patch on. Attach - # it only on PRs targeting master so release-branch PRs auto-pass. - - name: Compute Codecov flags - id: codecov-flags - shell: bash - env: - JOB_FLAGS: ${{ inputs.flags }} - EVENT_NAME: ${{ github.event_name }} - BASE_REF: ${{ github.base_ref }} - run: | - flags="$JOB_FLAGS" - if [ "$EVENT_NAME" = "pull_request" ] && [ "$BASE_REF" = "master" ]; then - flags="${flags:+$flags,}master-coverage" - fi - echo "value=$flags" >> "$GITHUB_OUTPUT" - - - name: Install gpg for Codecov validation - if: runner.os == 'Linux' - shell: bash - run: command -v gpg || sudo apt-get install -y gpg - - - name: Upload coverage to Codecov - uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7.0.0 - with: - flags: ${{ steps.codecov-flags.outputs.value }} - - - name: Upload coverage artifact - if: always() - uses: ./.github/actions/upload-coverage-artifact - with: - flags: ${{ inputs.flags }} - report-dir: ${{ inputs.report-dir }} diff --git a/.github/actions/install/branch-diff/action.yml b/.github/actions/install/branch-diff/action.yml index 1827f26d95..aafe85501e 100644 --- a/.github/actions/install/branch-diff/action.yml +++ b/.github/actions/install/branch-diff/action.yml @@ -7,7 +7,7 @@ inputs: runs: using: composite steps: - - uses: actions/cache@27d5ce7f107fe9357f9df03efb73ab90386fccae # v5.0.5 + - uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 with: path: ~/.npm key: ${{ github.workflow }}-branch-diff-3.1.1 diff --git a/.github/actions/node/setup/action.yml b/.github/actions/node/setup/action.yml index 745c7ba6fd..ef54965603 100644 --- a/.github/actions/node/setup/action.yml +++ b/.github/actions/node/setup/action.yml @@ -27,8 +27,10 @@ runs: } case "$VERSION" in eol) version=16 ;; - oldest) oldest_major=$(awk '/"node"/{match($0,/[0-9]+/);print substr($0,RSTART,RLENGTH);exit}' package.json) - version=$(node_version "$oldest_major") ;; + # Pinned to 18 rather than derived from package.json engines.node: CI keeps testing the + # oldest version we still exercise (18) even though the shipped engines floor is now >=22. + # See the Widen engines.node for CI step below, which restores >=18 on the 18/20 legs. + oldest) version=$(node_version 18) ;; maintenance) version=$(node_version 20) ;; active) version=$(node_version 24) ;; latest) version=${LATEST_VERSION:-$(node_version 26)} @@ -44,6 +46,22 @@ runs: node-version: ${{ steps.node-version.outputs.version }} registry-url: ${{ inputs.registry-url || 'https://registry.npmjs.org' }} + # The shipped package.json pins engines.node to the supported runtime range, but CI keeps + # running the full suite on Node 18/20. Widen the field to >=18 for this checkout so the + # runtime guard and the withVersions test helper don't bail and silently skip those jobs. + # Gate on the major so this never runs on the ancient runtimes the guardrails-unsupported + # job installs (those must keep seeing the shipped >=22 so the guard aborts), and so the + # script is only ever parsed by a Node version new enough to run it. + - name: Widen engines.node for CI + shell: bash + run: | + MAJOR=$(node -e "process.stdout.write(String(parseInt(process.versions.node)))") + if [ "$MAJOR" = "18" ] || [ "$MAJOR" = "20" ]; then + node scripts/ci/widen-engines-for-ci.js + else + echo "Leaving engines.node unchanged on Node $(node -v) (only widened on Node 18/20)." + fi + # Persist the resolved version so subsequent runs within this 60-minute window can reuse it. - name: Save resolved version if: steps.node-version-cache.outputs.cache-hit != 'true' diff --git a/.github/actions/upload-coverage-artifact/action.yml b/.github/actions/upload-coverage-artifact/action.yml index b29903dc8e..cbb4b9ed47 100644 --- a/.github/actions/upload-coverage-artifact/action.yml +++ b/.github/actions/upload-coverage-artifact/action.yml @@ -1,9 +1,9 @@ name: Upload Coverage Artifact -description: Upload coverage report as a GitHub Actions artifact for later collection +description: Upload a job's LCOV report as a GitHub Actions artifact for All Green to collect and group. inputs: flags: - description: Coverage flags — used as artifact identifier and passed to Datadog on upload + description: Coverage flags — the grouping key All Green folds matrix cells by required: true report-dir: description: Coverage report directory @@ -13,26 +13,38 @@ inputs: runs: using: composite steps: + # The reports live at `/node--