fix: Strip breadcrumb metadata on OOM to prevent silent event loss#2150
Open
spawnia wants to merge 8 commits into
Open
fix: Strip breadcrumb metadata on OOM to prevent silent event loss#2150spawnia wants to merge 8 commits into
spawnia wants to merge 8 commits into
Conversation
… serialization When an out-of-memory error occurs with large breadcrumbs (e.g. SQL queries with bindings), serializing the event requires more working memory than the 5MB headroom provided by ErrorHandler. This causes a secondary OOM during json_encode in PayloadSerializer, which is silently caught and discards the entire event. Detect OOM in FatalErrorListenerIntegration and replace breadcrumbs with metadata-stripped copies before capture. This preserves the full breadcrumb trail (type, category, level, timestamp, message) while freeing the bulky data fields that cause the serialization to exceed available memory. Also adds Scope::getBreadcrumbs() to enable reading breadcrumbs for transformation without introducing an event processor that would run after the large breadcrumbs are already copied to the event. Fixes getsentry/sentry-laravel#909 🤖 Generated with Claude Code
The re-add loop after stripping metadata used addBreadcrumb's default cap of 100, silently truncating breadcrumbs when the user configured a higher limit. Pass the actual count so all stripped breadcrumbs survive. 🤖 Generated with Claude Code
PHP 8.5 appends a stack trace to the fatal error output, requiring a separate EXPECTF block. Also remove unused $serialized variable from the php84 variant. 🤖 Generated with Claude Code
The FatalErrorException message has an "Error: " prefix, so the anchor from ErrorHandler's pattern would not match. No captures are needed since this is a boolean check. 🤖 Generated with Claude Code
getsentry#2150 (comment) 🤖 Generated with Claude Code
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit c30b256. Configure here.
The php84/ OOM tests used >= 80400, excluding PHP 8.4 itself. The stack trace output change only happened in 8.5, so these tests should run on 8.4 as well — matching the tests/phpt/php84/ convention of >= 80500. getsentry#2150 (comment) 🤖 Generated with Claude Code
Moves error_reporting() before require autoload.php so that OpenTelemetry's implicit-nullable deprecation notices on PHP 8.4/8.5 lowest deps don't pollute the PHPT expected output. 🤖 Generated with Claude Code
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Fixes getsentry/sentry-laravel#909
When an out-of-memory error occurs with large breadcrumbs (e.g. 100 SQL queries with ~100KB bindings each), the event is silently lost. Serialization needs ~15MB of working memory but only 5MB headroom is available after ErrorHandler raises the limit. The secondary OOM during
json_encodeis caught byinvokeListeners()and discarded.This fix detects OOM in
FatalErrorListenerIntegrationand strips breadcrumb metadata before capture — freeing the bulk of the memory while preserving the full breadcrumb trail (type, category, timestamp, message) for debugging.Design decisions
Why strip metadata instead of clearing all breadcrumbs? The breadcrumb timeline (which queries ran, in what order) is valuable for diagnosing OOM. The metadata (SQL bindings, HTTP bodies) is what carries the bulk (~99% of size) and is rarely needed for OOM diagnosis.
Why in the integration, not ErrorHandler or PayloadSerializer? The integration already has access to the exception message for OOM detection. Putting it in ErrorHandler would couple the low-level handler to Hub/Scope. Catching OOM in the serializer is unreliable since PHP's nested try/catch behavior during OOM is fragile.
Why modify the scope directly instead of using an event processor? The scope's breadcrumb objects hold references to the large metadata strings. These must be freed from PHP's heap before serialization to make room. An event processor runs after
applyToEventcopies breadcrumbs to the event — by then the scope still holds the originals in memory and the headroom is already consumed.Why add
Scope::getBreadcrumbs()? We need to read the breadcrumbs to build stripped copies before clearing. The scope had no getter —addBreadcrumbandclearBreadcrumbsexisted but you couldn't iterate the collection.🤖 Generated with Claude Code