From c1d78e4fe5c365b0f6de7b12a16ff6969dbf0492 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Tue, 29 Sep 2026 14:22:45 +0200 Subject: [PATCH 1/2] docs: Allow broad catches only at SDK boundaries Inside the SDK, catch the narrowest type. Broad catches are allowed only at boundaries where the thrown types are unknowable, must call rethrowIfFatal first, and must document what happens on failure. Align BUGBOT.md with AGENTS.md so Bugbot enforces the same policy. Co-Authored-By: Claude Opus 5.5 (1M context) --- .cursor/BUGBOT.md | 25 +++++++++++++++++------- AGENTS.md | 49 ++++++++++++++++++++++++----------------------- 2 files changed, 43 insertions(+), 31 deletions(-) diff --git a/.cursor/BUGBOT.md b/.cursor/BUGBOT.md index 5abe0d601f0..4525d5ff82e 100644 --- a/.cursor/BUGBOT.md +++ b/.cursor/BUGBOT.md @@ -13,15 +13,26 @@ rule file in [`.cursor/rules/`](rules) for the area the diff touches (`api`, `op - While we don't want to crash or hang the host application, we also don't want to leave the host application in a bad or unrecoverable state. Therefore catch the narrowest type the guarded code can throw. -- Existing broad catches like `catch (Throwable)` are legacy, not precedent. Where a broad catch is - genuinely unavoidable (an entry point running user code or third-party callbacks), it must call - `ExceptionUtils.rethrowIfFatal(t)` first and a code comment must say why the broad catch is - needed. +- Flag any new broad catch (`Throwable`, `Error`, `Exception`, `RuntimeException`) outside a + boundary where we can't know what the other side throws, e.g. public API entry points like + `captureException`, or Android framework calls that cross into another process + (`ContentResolver`, system services). Existing broad catches are legacy, not precedent. See the + Exception Handling section in `AGENTS.md`. +- A broad catch at a boundary must call `ExceptionUtils.rethrowIfFatal(t)` first and requires a + code comment saying why the catch is broad and what happens on failure (event dropped, feature + disabled, ...). +- A broad catch that wraps user code requires documentation, both in code and in sentry-docs, + explaining how the user is expected to discover that the code they wrote isn't working. + sentry-docs is a separate repository, so ask the author to link the docs PR. - Code probing for an optional `compileOnly` dependency must catch the specific `LinkageError` - subclass (`NoClassDefFoundError`, `NoSuchMethodError`, ...) only. + subclass (`NoClassDefFoundError`, `NoSuchMethodError`, ...). Flag a probe guarded only by a broad + catch with `rethrowIfFatal`: it rethrows every `LinkageError`, crashing the host app when the + dependency is missing. +- Logging is not handling: `options.getLogger()` is silent unless `debug` is enabled. When a catch + only logs, ask the author what the customer sees when it fires. - The SDK must never `captureException`/`captureMessage` for its own failures or for exceptions - thrown inside user callbacks (`beforeSend`, `beforeBreadcrumb`, `tracesSampler`, ...). Log via - `options.getLogger()` instead — capturing here loops. See + thrown inside user callbacks (`beforeSend`, `beforeBreadcrumb`, `tracesSampler`, ...) — capturing + here loops. See [Never capture your own exceptions](https://develop.sentry.dev/sdk/getting-started/principles/#never-capture-your-own-exceptions). - Flag `System.out`/`System.err`, `printStackTrace()`, and `android.util.Log` in SDK source; use `options.getLogger().log(...)`. diff --git a/AGENTS.md b/AGENTS.md index f447fc31512..989ec5c327f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -129,30 +129,31 @@ The repository is organized into multiple modules: ### Exception Handling -**Never introduce a new `catch (Throwable)`.** Catch the narrowest type the guarded code can -actually throw. The repository still contains many pre-existing broad catches; they are legacy, -not a precedent to follow. - -A broad catch swallows `OutOfMemoryError`, `StackOverflowError`, `ThreadDeath` and `LinkageError` — -conditions the JVM/ART cannot recover from and that leave the process in an undefined state — and -it hides real bugs in our own code behind a log line. - -"The SDK must never crash the host application" is not a reason to catch `Throwable`. That goal is -served by `io.sentry.util.ExceptionUtils.rethrowIfFatal`, which lets the non-recoverable throwables -through while leaving everything else for the caller to log or ignore: - -```java -try { - doSomethingRisky(); -} catch (Throwable t) { - ExceptionUtils.rethrowIfFatal(t); - options.getLogger().log(SentryLevel.ERROR, "Failed to do something risky", t); -} -``` - -Apply that pattern only where a broad catch is genuinely unavoidable — an entry point that runs -arbitrary user code or third-party callbacks. Everywhere else, name the exception types. Say in the -PR description why the broad catch is necessary. +Inside the SDK, **never add a broad catch** (`Throwable`, `Error`, `Exception`, +`RuntimeException`). Catch the narrowest type the guarded code can actually throw. The many +existing broad catches are legacy, not precedent: they hide real bugs in our own code and make it +appear that Sentry is broken. + +A broad catch is allowed only at a boundary where we can't know what the other side throws. Two +examples: + +- public API entry points like `captureException` +- Android framework calls that cross into another process (`ContentResolver`, system services) + +At a boundary: + +- Call `ExceptionUtils.rethrowIfFatal(t)` first so `OutOfMemoryError`, `StackOverflowError`, etc. + are never swallowed. It also rethrows `LinkageError`, so code probing an optional `compileOnly` + dependency must catch the specific subclass (`NoClassDefFoundError`, ...) before it. +- A code comment is required saying why the catch is broad and what happens on failure (event + dropped, feature disabled, ...). +- If wrapping user code with a broad catch, documentation is **required** explaining how the user + is expected to discover that the code they wrote isn't working. Both in code and in sentry-docs. +- Logging is not handling: `options.getLogger()` is silent unless `debug` is enabled, so a failure + that is only logged is invisible in production. + +Instrumentation that wraps user code is not a boundary for the user's own exceptions: let them +propagate unchanged. ### Concurrency From 1581f85b8582a99fdf6b634fb5b89593fcc71376 Mon Sep 17 00:00:00 2001 From: Nelson Osacky Date: Wed, 21 Oct 2026 13:11:56 +0200 Subject: [PATCH 2/2] Make some changes --- AGENTS.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 989ec5c327f..68a7bdfb74e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -129,6 +129,9 @@ The repository is organized into multiple modules: ### Exception Handling +The following section applies only in cases where the spec (sentry-docs) do not define an exception +handling strategy. + Inside the SDK, **never add a broad catch** (`Throwable`, `Error`, `Exception`, `RuntimeException`). Catch the narrowest type the guarded code can actually throw. The many existing broad catches are legacy, not precedent: they hide real bugs in our own code and make it @@ -139,8 +142,9 @@ examples: - public API entry points like `captureException` - Android framework calls that cross into another process (`ContentResolver`, system services) +- user callbacks like `beforeSendLogs` defined in the spec (sentry-docs) -At a boundary: +At a boundary follow the spec (sentry-docs) otherwise if no spec is defined for this case: - Call `ExceptionUtils.rethrowIfFatal(t)` first so `OutOfMemoryError`, `StackOverflowError`, etc. are never swallowed. It also rethrows `LinkageError`, so code probing an optional `compileOnly` @@ -148,7 +152,7 @@ At a boundary: - A code comment is required saying why the catch is broad and what happens on failure (event dropped, feature disabled, ...). - If wrapping user code with a broad catch, documentation is **required** explaining how the user - is expected to discover that the code they wrote isn't working. Both in code and in sentry-docs. + is expected to discover that the code they wrote isn't working. Both in the Javadoc and in sentry-docs. - Logging is not handling: `options.getLogger()` is silent unless `debug` is enabled, so a failure that is only logged is invisible in production.