Skip to content
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 29 additions & 2 deletions .cursor/BUGBOT.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,28 @@ Flag any of the following indicators or missing requirements.
If you find anything to flag, mention that you flagged this in the review because it was mentioned in this rules file.
Unless explicitly noted (e.g. in the `Testing Conventions` section), only flag the issues below in production code — ignore them in test files.

These rules operationalize the Sentry SDK [philosophy](https://develop.sentry.dev/sdk/getting-started/philosophy/) and [principles](https://develop.sentry.dev/sdk/getting-started/principles/): protect customer apps and data, prefer safe defaults, keep the base SDK lean, stay compatible, and never let SDK or callback failures become Sentry traffic or host crashes.

Keep reviews high-signal. Prefer actionable, high-confidence findings over speculative warnings or drive-by refactors unrelated to the diff.

## Critical Issues to Flag

### Security Vulnerabilities

- Exposed secrets, API keys, tokens or credentials in code or comments
- Exposed secrets, API keys, tokens, DSNs, or credentials in code, comments, logs, configs, or examples
Comment thread
sentry-junior[bot] marked this conversation as resolved.
Outdated
- Unsafe use of `eval()`, `Function()`, or `innerHTML`
- Unsafe regular expressions that could cause ReDoS attacks
- PII or sensitive data attached by auto-instrumentation without an explicit opt-in (`sendDefaultPii` or equivalent). Flag new default logging/sending of request/response bodies, full URLs with query secrets, file paths, or device identifiers.
Comment thread
sentry-junior[bot] marked this conversation as resolved.
Outdated
- Large or sensitive attachments enabled by default, or attachments lacking size limits / backoff
Comment thread
sentry-junior[bot] marked this conversation as resolved.
Outdated
- Diagnostics, sampling overrides, verbose logging, or feature flags accidentally enabled in production defaults

### Breaking Changes

- Public API changes without proper deprecation notices
- Removal of publicly exported functions, classes, or types. Internal removals are fine!
- Changes to function signatures in public APIs
- Silent changes to defaults, sampling, or feature toggles that affect existing apps without migration notes
- Raising minimum runtime/browser/Node support, or dropping a supported platform, without an explicit docs/changelog/migration callout

## SDK-relevant issues

Expand All @@ -28,6 +37,14 @@ Unless explicitly noted (e.g. in the `Testing Conventions` section), only flag t
- Large bundle size increases in browser packages. Sometimes they're unavoidable but flag them anyway.
- Flag top-level side effects (function calls, mutations of module-level state, IIFEs) in modules that are reachable from a package's public entry points. Side effects defeat tree-shaking and inflate bundles for users who don't import the affected code. Pure exports (constants, classes, factory functions, side-effect-free declarations) are fine.

### Trust, defaults, and dependencies

- Never let SDK init or instrumentation failures crash or brick the host application. Prefer graceful degrade / no-op on unsupported or outdated environments; if that is impossible, the break must be obvious at install/build time, not at runtime.
- Flag new runtime dependencies on the base SDK path. Integration-only optional dependencies are fine when justified; baseline deps increase license, maintenance, and supply-chain surface.
- Flag new required configuration or integrations left disabled by default when auto-enable would give a better out-of-the-box experience without surprising side effects.
- Flag heavy in-SDK business logic that permanently reshapes wire-format data when collecting rawer data and leaving transformation to the server would do.
- SDKs must never `captureException` / `captureMessage` (or equivalent) for exceptions thrown inside the SDK itself or inside user callbacks such as `beforeSend` / `before_send`, `tracesSampler`, or similar hooks. Swallow gracefully and emit an error-level SDK log instead — capturing here can loop and take down the process. See [Never capture your own exceptions](https://develop.sentry.dev/sdk/getting-started/principles/#never-capture-your-own-exceptions).

### Auto instrumentation, SDK integrations, Sentry-specific conventions

- When calling any `startSpan` API (`startInactiveSpan`, `startSpanManual`, etc), always ensure that the following span attributes are set:
Expand All @@ -40,6 +57,7 @@ Unless explicitly noted (e.g. in the `Testing Conventions` section), only flag t
- When calling `captureException`, always make sure that the `mechanism` is set:
- `handled`: must be set to `true` or `false`
- `type`: must be a proper identifier (i.e. identify the integration and part in the integration that caught the exception). The value should follow the same naming convention as `SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN`, and align with the surrounding span's origin when one exists.
- For automated structured logs, always set `sentry.origin` with a proper origin (same character rules as span origins).
- When calling any `startSpan` API, check how errors in the instrumented code are handled:
- Generally, errors in instrumented code should be allowed to bubble up so the end user can handle them. If they remain unhandled, they will eventually be captured by Sentry through the SDK's global error handlers — so instrumentation code should typically **not** call `captureException` itself.
- Only consider calling `captureException` if the instrumentation prevents errors from bubbling up (e.g. by swallowing them in a `try/catch` or an error event listener). Doing so is generally discouraged — prefer to let the error propagate instead.
Expand All @@ -64,8 +82,9 @@ Unless explicitly noted (e.g. in the `Testing Conventions` section), only flag t

- When reviewing a `feat` PR, check if the PR includes at least one integration or E2E test. If neither is present, flag it and recommend adding one.
- When reviewing a `fix` PR, check if the PR includes at least one unit, integration or E2E test that covers the regression this PR fixes. The test should fail without the fix and pass with it. If you cannot tell from the diff whether this is the case, ask the author to confirm. If no tests are present, flag it and recommend adding one.
- Check that tests actually test the newly added behaviour.
- Check that tests actually test the newly added behaviour. Tests must prove user-visible or SDK behavior, not merely coverage or "did not throw".
For instance, when checking on sent payloads by the SDK, ensure that the newly added data is asserted thoroughly.
Hollow tests that look correct but assert nothing meaningful should be flagged.
- Flag usage of `expect.objectContaining` and other relaxed assertions, when a test expects something NOT to be included in a payload but there's no respective assertion.
- Flag usage of conditionals in one test and recommend splitting up the test for the different paths.
- Flag usage of loops testing multiple scenarios in one test and recommend using `(it)|(test).each` instead.
Expand All @@ -81,3 +100,11 @@ Unless explicitly noted (e.g. in the `Testing Conventions` section), only flag t
- When any `setTimeout` or `setInterval` timers are started in a code path that can end up in server runtime packages (e.g. `@sentry/core` or `@sentry/node`), flag if neither `timeout.unref()` nor `safeUnref()` are called.
Not unref'ing a timer can keep CLI-like applications or node scripts from exiting immediately, due to the process waiting on timers started by the SDK.
- Flag Node-only imports (`fs`, `path`, `child_process`, `os`, `net`, `http`, `https`, `node:*`, etc.) in code paths that ship to non-Node runtimes (`@sentry/browser`, `@sentry/cloudflare`, `@sentry/deno`, `@sentry/bun`, or shared code in `@sentry/core` / `@sentry/browser-utils` reachable from those entry points). When the dependency is unavoidable, isolate it behind a runtime check, dynamic `import()`, or a Node-only entry point — never at the top level of a cross-runtime module.

## What NOT to Flag

- Pure style or formatting that linters/formatters already own (Oxlint, Oxfmt, etc.)
- Speculative refactors or "improvements" with no clear user benefit or linked motivation
- Idiomatic monkeypatching / low-level hooks solely because they are brittle — only flag when unsafe, non-idempotent, or harmful to the host app
Comment thread
sentry-junior[bot] marked this conversation as resolved.
Outdated
- Test-only issues unless covered by `Testing Conventions`
- Conventional commit / PR title format when CI already validates it
Loading