feat(cli): add Sentry error telemetry + stabilize flaky stat tests#200
feat(cli): add Sentry error telemetry + stabilize flaky stat tests#200designcode wants to merge 6 commits into
Conversation
The org-wide `stat` command issues a single getStats request against a live, eventually-consistent gateway; a transient network blip made it exit non-zero (network/general) and fail two overall-stats tests that assert exitCode === 0. Add retry: 2 to the CLI vitest config, mirroring packages/storage, so transient failures are absorbed while deterministic failures still fail every attempt. Assisted-by: Opus 4.8 via Claude Code
Report crashes plus unexpected (general) and network errors to Sentry so we can debug customer issues, with the command, error category, exit code, CLI version, and platform attached. Expected user-facing conditions (auth, permission, not_found, rate_limit) are not reported. Telemetry is initialized in setupErrorHandlers() and is a strict no-op unless a DSN is configured; it stays off in dev/test and on opt-out (TIGRIS_NO_TELEMETRY / DO_NOT_TRACK) and never throws into the command path. Secrets (access keys, tokens, credential flags) and the machine hostname are scrubbed in beforeSend. exitWithError() stays synchronous so the never-returning contract that command code relies on is preserved; handled errors are captured best-effort. The global crash handlers (uncaught/unhandled) capture and await a bounded flush before exiting, so crashes are delivered reliably. Validated under both the npm build and the bun-compiled binary (brew). Also reformat two test files and bump the biome schema to 2.5.4 to satisfy the pre-commit formatter (pre-existing drift from an earlier biome bump). Assisted-by: Opus 4.8 via Claude Code
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Greptile SummaryThis PR adds error telemetry and improves CLI test reliability. The main changes are:
Confidence Score: 4/5Invocation telemetry can disclose user and customer-resource data and should be narrowed before merging.
packages/cli/src/utils/telemetry.ts
|
| Filename | Overview |
|---|---|
| packages/cli/src/utils/telemetry.ts | Adds Sentry initialization, filtering, context, redaction, capture, and flushing; arbitrary argument values remain in invocation context. |
| packages/cli/src/cli-core.ts | Initializes telemetry and flushes captured crashes before preserving classified CLI exits. |
| packages/cli/src/utils/exit.ts | Adds synchronous handled-error capture while preserving the existing immediate-exit contract. |
| packages/cli/vitest.config.ts | Adds two retries across the CLI test suite for transient live-gateway failures. |
| packages/cli/package.json | Adds Sentry Node SDK as a runtime dependency. |
Reviews (1): Last reviewed commit: "feat(cli): add Sentry error telemetry" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| arch: process.arch, | ||
| }); | ||
| Sentry.setContext('invocation', { | ||
| command: scrubArgv(process.argv.slice(2)).join(' '), |
There was a problem hiding this comment.
sendDefaultPii: false does not sanitize context added by this code. scrubArgv() removes known credentials but still sends values such as notification usernames, local paths, bucket names, and object keys; for example, --username=user@example.com reaches contexts.invocation.command unchanged on every captured error.
The CLI tsconfig relies on automatic @types discovery for Node globals (process, console). `tsc` run from the package resolves the root-hoisted @types/node by walking up, but the editor's TypeScript server does not pick it up through pnpm's symlinked workspace layout, so it reported "Cannot find name 'process'/'console'" (and a cascade of any/unknown) even though the build was green. Declare `types: ["node"]` so the TS server and tsc agree. tsconfig.binary.json extends this config, so it inherits the setting. Assisted-by: Opus 4.8 via Claude Code
Address PR review: the invocation context sent the full argv (only known secret flags redacted), so positionals and non-secret flag values — bucket names, object keys, file paths, emails — reached Sentry, and contexts are not run through beforeSend. Capture only flag names now (invocationFlags), dropping every value and positional. Also drop Sentry default integrations that capture unscrubbed user data: LocalVariables/LocalVariablesAsync (local variable values, e.g. credentials, into stack frames) and Console/ChildProcess (breadcrumbs carrying printed output and spawned command lines). Assisted-by: Opus 4.8 via Claude Code
…e tests Integration teardowns removed tracked buckets in push order, but a fork is pushed after its source and a source cannot be removed while a dependent fork still exists — so the source delete failed (error unchecked) and the source bucket leaked. Delete in reverse (LIFO) so forks go before sources (bucket-create and agent-kit checkpoint/restore suites). Also disable the CLI fork rebase/merge tests: those operations snapshot the fork, and the snapshots block the fork's teardown, leaking the fork and its source bucket. Assisted-by: Opus 4.8 via Claude Code
redactSecrets used one replacer that treated the second arg as a capture
group, but for the group-less patterns (JWT, Bearer, AKIA) that arg is
the match offset — so a secret found mid-string was replaced with
`<offset>[redacted]` (e.g. `9[redacted]`). The secret was still removed,
but the scrubbed text was corrupted, and the tests missed it because they
only asserted toContain('[redacted]').
Only treat the replacer arg as a prefix when it is a string, and assert
exact output in the tests so the offset injection can't regress.
Assisted-by: Opus 4.8 via Claude Code
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a855c35. Configure here.
| return argv | ||
| .filter((arg) => arg.startsWith('-')) | ||
| .map((arg) => arg.split('=')[0]); | ||
| } |
There was a problem hiding this comment.
Crash events omit command context
Medium Severity
Crash telemetry cannot identify which command failed. invocationFlags drops all positionals, so command names never reach the invocation context, and reportCrashAndExit calls captureError without a command tag. Handled errors can still get command from MessageContext, but the reliably flushed crash path — the main debugging signal — has no command identity.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit a855c35. Configure here.
| captureError(error, { | ||
| category: classified.category, | ||
| command: context?.command, | ||
| }); |
There was a problem hiding this comment.
Exit code never sent to Sentry
Low Severity
exitWithError has classified.exitCode available but never passes it into captureError, and captureError has no exit-code field or tag. The changeset and PR both claim events include exit code, so that context is missing on both handled and crash reports.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit a855c35. Configure here.
1 participant
What
Two related reliability changes to the CLI, in separate commits.
1. Stabilize flaky
statintegration tests (test(cli))The org-wide
statcommand issues a singlegetStatsrequest against a live, eventually-consistent gateway. A transient network blip made it exit non-zero (network/general), failing two overall-stats tests that assertexitCode === 0(expected 5 to be 0/expected 1 to be 0). The CLI vitest config had noretry, unlikepackages/storagewhich usesretry: 2for exactly this reason.retry: 2topackages/cli/vitest.config.ts. Deterministic failures still fail all attempts, so real regressions aren't masked.2. Add Sentry error telemetry (
feat(cli))Reports errors to Sentry so we can debug customer issues.
generalandnetworkerrors. Expected user-facing conditions (auth,permission,not_found,rate_limit) are not reported.exitWithError()stays synchronous (preserves thenevercontract command code relies on), so handled errors are captured best-effort. The global crash handlers capture +await flushbefore exit, so crashes are delivered reliably.sendDefaultPii: false;beforeSendscrubs secrets (access keys, tokens, credential flags) and the machine hostname; argv values after credential flags are redacted. Unit-tested (test/utils/telemetry.test.ts, 12 tests).TIGRIS_NO_TELEMETRY=1/DO_NOT_TRACK=1). Never throws into the command path.Also folds in a pre-existing biome 2.5.4 format drift fix (two test files +
biome.jsonschema bump) so the pre-commit formatter passes.Validation
tsc, biome, and unit tests pass.flushreturned true).Follow-ups (not in this PR)
try/catchblocks insrc/lib) sogeneral/networkerrors flush reliably too.🤖 Generated with Claude Code
Note
Medium Risk
Telemetry touches every exit/crash path and ships a public DSN, but it is designed as a no-op on failure and scrubs secrets; handled-error delivery remains best-effort until a follow-up boundary refactor.
Overview
Adds opt-in Sentry error reporting for
@tigrisdata/clivia a newtelemetrymodule and@sentry/node. Crashes (uncaughtException/unhandledRejection) are captured and flushed before exit; handled failures go throughexitWithErroras best-effort capture (sync exit unchanged). Onlygeneralandnetworkcategories are reported on the handled path; expected auth/permission/not-found noise is skipped.Events get CLI version, platform, command/category tags, and flag names only from argv (no positionals or flag values).
beforeSendstrips hostname and scrubs tokens/keys; several Sentry integrations are disabled to avoid PII and conflicting exit handling. Telemetry is off in test/dev, without a DSN, or whenTIGRIS_NO_TELEMETRY=1/DO_NOT_TRACK=1; a build-time DSN is embedded (overridable viaTIGRIS_SENTRY_DSN).Test reliability: CLI vitest gets
retry: 2for live-gateway flakes (e.g.stat). Integration cleanups delete buckets LIFO when forks depend on sources; rebase/merge fork lifecycle tests areit.skipuntil snapshot teardown is fixed. Minor Biome 2.5.4 schema/format touch-ups.Reviewed by Cursor Bugbot for commit a855c35. Bugbot is set up for automated code reviews on this repo. Configure here.