Skip to content

feat: Integrate Sentry for error reporting#650

Open
Ellie Gui (eguicf) wants to merge 9 commits into
mainfrom
egui/sentry-integration
Open

feat: Integrate Sentry for error reporting#650
Ellie Gui (eguicf) wants to merge 9 commits into
mainfrom
egui/sentry-integration

Conversation

@eguicf

@eguicf Ellie Gui (eguicf) commented Jun 29, 2026

Copy link
Copy Markdown
Member

Summary of Changes

  • closes Integrate Sentry for client-side error reporting #478
  • sentry.ts — Sentry lifecycle: consent-gated initSentry (short-circuits on DO_NOT_TRACK / no DSN), captureException, closeSentry, resolveSentryDsn; PII off, beforeSend redaction, release/transport/connections tags.
  • sentry-redact.ts — scrubs auth headers, API key/secret, SASL passwords, and YAML secrets from outbound events.
  • index.ts — initializes Sentry after config load (so consent is known), captures startup errors, flushes on shutdown; adds resolveDoNotTrack (one switch for analytics + errors) and connectionTypes context.
  • server.ts — reports tool-handler throws via captureError(error, toolName).
  • node-deps.ts — adds the stubbable sentry wrapper and SENTRY_DSN.
  • build-config.ts + inject-build-config.mjs — bake SENTRY_DSN into the build at pack time.
  • release-version.yml — pulls SENTRY_DSN from Vault and uploads source maps at release.
  • Tests, docs updated/added accordingly.

Manual testing instructions

Sentry config lives in Vault at stag/kv/semaphore/mcpconfluent/telemetry (fields SENTRY_DSN, SENTRY_AUTH_TOKEN; CI reads them via vault-get-secret). Fetch the DSN:

vault kv get -field=SENTRY_DSN stag/kv/semaphore/mcpconfluent/telemetry

Events land here: https://confluent.sentry.io/issues/?project=4511649921761280&query=is%3Aunresolved&statsPeriod=7d

  1. Live (Sentry SDK): save this file locally/mcp-confluent/scripts/sentry-smoke.mjs
#!/usr/bin/env node

// Manual smoke test for Sentry error reporting. Sends REAL events to the DSN
// you provide, so reviewers can confirm end-to-end delivery without a release.
// The unit suite (`pnpm run test:unit`) already covers the logic; this is the
// live "does it actually reach Sentry" check.
//
// Usage (from repo root, after `pnpm run build`):
//   SENTRY_DSN="<project dsn>" node scripts/sentry-smoke.mjs [capture|uncaught|optout]
//
//   capture  (default) explicit captureException + tags + secret redaction
//   uncaught            unhandled throw caught by the global handler (exits 1)
//   optout               DO_NOT_TRACK path: init short-circuits, no event sent

import { dirname, join } from "node:path";
import { fileURLToPath } from "node:url";

const dsn = process.env.SENTRY_DSN;
if (!dsn) {
  console.error(
    "Set SENTRY_DSN, e.g. SENTRY_DSN=... node scripts/sentry-smoke.mjs",
  );
  process.exit(1);
}

const distPath = join(
  dirname(fileURLToPath(import.meta.url)),
  "..",
  "dist",
  "confluent",
  "sentry.js",
);
let sentry;
try {
  sentry = await import(distPath);
} catch {
  console.error("dist not found — run `pnpm run build` first.");
  process.exit(1);
}
const { initSentry, captureException, closeSentry } = sentry;

const mode = process.argv[2] ?? "capture";
const stamp = new Date().toISOString();

initSentry({
  doNotTrack: mode === "optout",
  dsn,
  release: "smoke",
  transports: ["stdio", "http"],
  connectionTypes: ["oauth", "direct"], // unsorted on purpose -> tag "direct,oauth"
});

if (mode === "optout") {
  captureException(new Error(`TESTING optout ${stamp} — should NOT appear`));
  await closeSentry(5000);
  console.log("opt-out: init short-circuited; NO event should reach Sentry.");
} else if (mode === "uncaught") {
  console.log("throwing uncaught — process exits 1 after Sentry flushes…");
  setTimeout(() => {
    throw new Error(`TESTING uncaught ${stamp}`);
  }, 200);
} else {
  captureException(
    new Error(
      `TESTING capture ${stamp} — Authorization: Bearer FAKE_REDACT_ME_123`,
    ),
    { tags: { toolName: "list-topics" } },
  );
  const ok = await closeSentry(5000);
  console.log(
    ok
      ? "Sent. Sentry Issue 'TESTING capture …': tags connections=direct,oauth, transport=http,stdio, toolName=list-topics; Bearer -> [REDACTED]."
      : "Flush timed out — check DSN/network.",
  );
}

build, then run the smoke tool in its three modes:

pnpm run build
SENTRY_DSN="<dsn>" node scripts/sentry-smoke.mjs           # explicit capture
SENTRY_DSN="<dsn>" node scripts/sentry-smoke.mjs uncaught  # unhandled throw
SENTRY_DSN="<dsn>" node scripts/sentry-smoke.mjs optout    # opted out

Expected:

  • capture → new Issue TESTING capture … with tags connections=direct,oauth, transport=http,stdio, toolName=list-topics; the Bearer … token shows [REDACTED].
  • uncaught → new Issue TESTING uncaught …; the process prints the stack and exits 1.
  • optoutno new Issue; logs Error reporting disabled.
  1. Automated: pnpm run test:unit Expected: all unit tests pass.

  2. Consent + real server — bakes the DSN into dist (what a published build has) and starts the real server, to confirm initSentry runs in the actual startup path and honors the switch. (inject-build-config.mjs rewrites dist/build-config.js with the env values — the same step pnpm pack runs at release; here it makes a dev build behave like a published one.)

    pnpm run build
    SENTRY_DSN="<dsn>" node scripts/inject-build-config.mjs        # bakes SENTRY_DSN into dist/build-config.js
    node dist/index.js --config ./config.yaml                      # start with consent
    DO_NOT_TRACK=true node dist/index.js --config ./config.yaml    # start opted out
    pnpm run build                                                 # reset dist (empty DSN)

    Expected:

    • inject-build-config.mjs → prints inject-build-config: SENTRY_DSN set.
    • consent start → logs Error reporting enabled.
    • opted-out start → logs Error reporting disabled.

TODO

  • create a slack channel and integrate with sentry to receive alerts
  • set alert rules

Pull request checklist

Please check if your PR fulfills the following (if applicable):

Tests

  • Added new
  • Updated existing
  • Deleted existing

Release notes

  • Does anything in this PR need to be mentioned in the user-facing CHANGELOG?

Copilot AI review requested due to automatic review settings June 29, 2026 23:59

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds an opt-out–governed Sentry integration to report MCP server runtime errors (tool handler throws, uncaught exceptions, unhandled rejections), alongside updates to telemetry docs, build-time configuration injection, and the release pipeline to upload source maps.

Changes:

  • Add Sentry initialization/capture/flush plumbing and wire it into tool-call error paths and top-level startup error handling.
  • Add redaction logic for outbound Sentry events and unit tests covering initialization, flushing, and redaction behavior.
  • Update docs/config/changelog and release packaging steps to support baking a DSN at pack time and uploading source maps.

Reviewed changes

Copilot reviewed 17 out of 18 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
telemetry.md Updates telemetry documentation to include Sentry error reporting and opt-out behavior.
src/mcp/server.ts Adds a captureError hook to report tool handler throws.
src/mcp/server.test.ts Adds test coverage for captureError being invoked with the tool name.
src/index.ts Computes a unified doNotTrack flag, initializes Sentry, captures startup errors, and flushes on shutdown.
src/index.test.ts Adds tests for resolveDoNotTrack() and flushing Sentry during shutdown.
src/confluent/sentry.ts Introduces the Sentry wrapper module (init/capture/close) with redaction hook.
src/confluent/sentry.test.ts Adds unit tests for Sentry init gating, capture, and flush behavior.
src/confluent/sentry-redact.ts Adds deep redaction of sensitive fields/patterns in outbound Sentry events.
src/confluent/sentry-redact.test.ts Adds tests validating secret scrubbing and preservation of non-secret content.
src/confluent/node-deps.ts Wraps @sentry/node behind node-deps for ESM stubbability and build-config access.
src/build-config.ts Adds SENTRY_DSN build-time constant.
scripts/inject-build-config.mjs Injects SENTRY_DSN into packed dist/build-config.js and logs injection status.
README.md Updates the Telemetry section to mention Sentry and YAML opt-out.
package.json Adds @sentry/node dependency and @sentry/cli devDependency.
CONFIGURATION.md Updates opt-out docs to clarify it disables both analytics and Sentry reporting.
CHANGELOG.md Adds an Unreleased entry describing new Sentry error reporting behavior and privacy properties.
.semaphore/release-version.yml Fetches DSN/auth token from Vault and uploads sourcemaps/releases via sentry-cli during release.
pnpm-lock.yaml Locks new Sentry dependencies (reviewed only at the top-level via package.json).
Files not reviewed (1)
  • pnpm-lock.yaml: Generated file

Comment thread telemetry.md Outdated
Comment thread README.md Outdated
Comment thread src/confluent/sentry-redact.ts Outdated
Comment thread src/confluent/sentry.ts
Comment thread src/index.ts
@eguicf Ellie Gui (eguicf) changed the title feat: Integrate Sentry for client-side error reporting feat: Integrate Sentry for error reporting Jun 30, 2026
@eguicf Ellie Gui (eguicf) marked this pull request as ready for review June 30, 2026 14:28
@eguicf Ellie Gui (eguicf) requested review from a team as code owners June 30, 2026 14:28
@bcscc Bernie Chen (bcscc) self-assigned this Jun 30, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 17 out of 18 changed files in this pull request and generated 2 comments.

Files not reviewed (1)
  • pnpm-lock.yaml: Generated file

Comment thread src/mcp/server.ts
Comment thread src/confluent/sentry.ts
@sonarqube-confluent

Copy link
Copy Markdown

@eguicf

Ellie Gui (eguicf) commented Jul 1, 2026

Copy link
Copy Markdown
Member Author

Bernie Chen (@bcscc) Thank you for reviewing the PR, and the valid secure concern about SENTRY_AUTH_TOKEN! After verify, Sentry auth token should not be exposed in the published package.

below two different values, handled differently:

  • SENTRY_DSN is intentionally baked into the published package (dist/build-config.js) and will be visible on npm, like the existing TELEMETRY_WRITE_KEY.
  • SENTRY_AUTH_TOKEN is never shipped. It exists only as a transient CI env var that sentry-cli uses to upload source maps at release time.

Why it can't leak, by construction:

  • scripts/inject-build-config.mjs bakes only TELEMETRY_WRITE_KEY and SENTRY_DSN — it never references SENTRY_AUTH_TOKEN.
  • package.json files ships only dist/ (+ examples). Env vars aren't files, so they can't be packed. Exposure depends on "written into a shipped file," not "present in the env."
  • The token comes from Vault and is consumed by sentry-cli at release time only — not in the repo, not part of runtime.

Proof it's not exposed (packed the real tarball with a fake token in the env, then searched the whole package):

SENTRY_DSN=<real> SENTRY_AUTH_TOKEN=FAKE_sntrys_xxx pnpm pack
tar -xzf confluentinc-mcp-confluent-*.tgz -C /tmp/pkgcheck
grep -rni "sntrys_\|AUTH_TOKEN" /tmp/pkgcheck/package/   # → no matches
cat  /tmp/pkgcheck/package/dist/build-config.js          # → SENTRY_DSN present, token absent

Proof it's actually used:

# A. Add a temporary throw at the top of a handler's handle(), e.g.
#    src/confluent/tools/handlers/diagnostics/list-configured-connections-handler.ts:
#      throw new Error("test sentry - sourcemap e2e");   // TEMP

# B. Build (compiles the throw), then bake the DSN (build resets it to empty)
pnpm run build
SENTRY_DSN="<dsn>" node scripts/inject-build-config.mjs
VERSION=$(jq -r .version package.json)          # e.g. 1.5.0

# C. Upload source maps for THIS build (needs the auth token — fails if invalid)
SENTRY_AUTH_TOKEN="<token>" SENTRY_ORG=confluent SENTRY_PROJECT=mcp-confluent \
 pnpm exec sentry-cli sourcemaps upload --release "$VERSION" --url-prefix '~/dist' dist

# D. Run under the Inspector
npx @modelcontextprotocol/inspector node dist/index.js -c ./clicktest-partial.yaml
#    UI → Tools → login using oauth → list-configured-connections → Run   (throws immediately)

Then open that Issue in Sentry and check the stack trace.

@eguicf Ellie Gui (eguicf) changed the base branch from main to v1.5.x July 1, 2026 18:04
@eguicf Ellie Gui (eguicf) changed the title feat: Integrate Sentry for error reporting [v1.5.x] feat: Integrate Sentry for error reporting Jul 1, 2026
@eguicf Ellie Gui (eguicf) changed the title [v1.5.x] feat: Integrate Sentry for error reporting feat: Integrate Sentry for error reporting Jul 7, 2026
@eguicf Ellie Gui (eguicf) changed the base branch from v1.5.x to main July 7, 2026 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Integrate Sentry for client-side error reporting

3 participants