Skip to content

fix(cli): stop error-tracking noise from duplicate-version uploads - #2934

Merged
riderx merged 2 commits into
mainfrom
posthog-self-driving/fixcli-stop-error-tracking-version-x-3f6676
Aug 8, 2026
Merged

fix(cli): stop error-tracking noise from duplicate-version uploads#2934
riderx merged 2 commits into
mainfrom
posthog-self-driving/fixcli-stop-error-tracking-version-x-3f6676

Conversation

@posthog-eu

@posthog-eu posthog-eu Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Who it hurts: the team, not the user. A duplicate-version bundle upload — a normal outcome the user fixes by bumping the version — still minted an error-tracking $exception issue on every run, crowding out real CLI crashes.
  • Last week's fix added CliUserError + a type filter in shouldCapturePosthogException, but Version X already exists was never wired into it: uploadFail threw a bare Error, which the message-substring filter never matched.
  • The fingerprint made it worse — it hashed the minified top-frame symbol and the full install path, so the same abort split into a fresh issue per npx cache hash, bunx, pnpm store, and release.

Two small changes:

Area Before After
Upload aborts uploadFail throws bare Error → captured throws CliUserError → filtered by type
Plan-upgrade checks + app add bare Error → captured CliUserError → filtered
Fingerprint fn:kind:name:symbol:filename:status fn:kind:name:status
Graceful-exit warning names --silent-fail (does not exist) names --version-exists-ok

Behaviour kept intact: non-zero exit, the printed message, and the trackCommandFailed analytics events. Only the $exception capture goes away.

Why drop the frame instead of normalizing it: the CLI ships as one minified dist/index.js, so the top-frame symbol is renamed on every release and every frame's filename is the same install-path-prefixed index.js. Neither carries stable, discriminating signal — the command path, error kind, error name, and exit status do.

Test plan

  • bun test/test-posthog-exception.mjs — added cases: CliUserError('Version X already exists') and Upload cancelled by user are not captured; fingerprint no longer contains the symbol or filename.
  • bun test/test-init-app-conflict.mjsisAppAlreadyExistsError still matches the reworded app add error (id moved to context, "already exists" kept in the message).
  • bun test/test-plan-validation.mjs, bun test/test-trial-warning.mjs, bun test/test-analytics-error-category.mjs, bun test/test-app-created-source.mjs — all pass.
  • tsgo --noEmit and oxlint clean.

Screenshots

Not applicable — no visible CLI output changes except the corrected --version-exists-ok flag name in one warning line.

Checklist

  • My code follows the code style of this project and passes bun run lint:backend && bun run lint.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • My change has adequate E2E test coverage.
  • I have tested my code manually, and I have provided steps how to reproduce my tests

Created with PostHog Desktop from this inbox report.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Review in cubic

Duplicate-version uploads (`Version X already exists`) and other deliberate
upload aborts still opened `$exception` issues, because `uploadFail` threw a
bare `Error` that the message-substring filter never matched.

- `uploadFail`, the upload path's single failure chokepoint, now throws
  `CliUserError`, so `shouldCapturePosthogException` filters the whole path
  by type instead of by message. Same treatment for the plan-upgrade checks
  and `app add`. Non-zero exit and analytics events are unchanged.
- Drop the top-frame symbol and filename from the exception fingerprint. The
  CLI ships as one minified `dist/index.js`, so the symbol is renamed each
  release and the filename is the full install path — which splits one bug
  into a fresh issue per npx/bunx/pnpm/sandbox location and per release.
- Fix the graceful-exit warning to name the real flag `--version-exists-ok`
  instead of the non-existent `--silent-fail`.

Generated-By: PostHog Code
Task-Id: e2d8341d-2959-4d2c-852c-7173ea0705a7
@codspeed-hq

codspeed-hq Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 43 untouched benchmarks
⏩ 2 skipped benchmarks1


Comparing posthog-self-driving/fixcli-stop-error-tracking-version-x-3f6676 (a08f6f2) with main (10f1f4a)2

Open in CodSpeed

Footnotes

  1. 2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

  2. No successful run was found on main (034dd27) during the generation of this report, so 10f1f4a was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

Keep uploadCancel() from main (warn + CliUserError) and retain both
PostHog exception filter tests for duplicate-version and cancel paths.

Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
@cursor
cursor Bot temporarily deployed to deepsec-pr August 8, 2026 17:20 Inactive
@riderx
riderx merged commit 2d78f79 into main Aug 8, 2026
60 checks passed
@riderx
riderx deleted the posthog-self-driving/fixcli-stop-error-tracking-version-x-3f6676 branch August 8, 2026 17:22
cursor Bot pushed a commit that referenced this pull request Aug 8, 2026
Keep main's install-path fingerprint comments and both CliUserError skip
assertions (not-logged-in from this PR, duplicate-version from #2934).

Co-authored-by: Martin DONADIEU <martindonadieu@gmail.com>
@sonarqubecloud

sonarqubecloud Bot commented Aug 8, 2026

Copy link
Copy Markdown

posthog-eu Bot added a commit that referenced this pull request Aug 9, 2026
A transient database failure in `getUserIdFromApikey` returned null, which the
files service reported as `400 user_not_found`. The CLI showed users a bad-key
message and opened an error-tracking issue for each failure.

Backend:
- `getUserIdFromApikey` now rethrows a query failure instead of returning null,
  so a backend blip surfaces as a 500, not a missing user.
- `checkWriteAppAccess` returns `401 user_not_found` (was 400), matching how
  `invalid_apikey` behaves on the same route.

CLI:
- Add a shared `buildTusUploadError` helper that turns a tus failure into a real
  Error carrying the HTTP status, backend message, and request id. `uploadTUS`
  and the delta path (`partial.ts`) both use it, so the delta path no longer
  rejects the raw tus blob that leaked the URL and per-file object key.
- The attached `.status` and the new `user_not_found` marker let
  `isExpectedUserError` classify the auth failure and skip error tracking.

Merging this fix cuts a CLI release that also carries the earlier fixes (#2885,
#2933, #2934, #2947) that no released build contains yet.

Generated-By: PostHog Code
Task-Id: c66eba6b-af49-4627-84ac-2c007ed83bc0
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.

2 participants