MaxBreadcrumbs setting is not respected and leads to failures to ingest events

[riot-pmaconi]

Environment

How do you use Sentry? Sentry SaaS (sentry.io) or self-hosted/on-premise (which version?)

Which version of the SDK?

0.18

How did you install the package? (Git-URL, Assetstore)

git

Which version of Unreal?

5.4.4

Is this happening in Unreal (editor) or on a player like Android, iOS, Windows?

All desktop platforms at least

Steps to Reproduce

1. Call SentrySubsystem::AddBreadcrumb or SentrySubsystem::AddBreadcrumbWithParams more times than the MaxBreadcrumbs config, which is 100 by default
2. Send a Sentry event

Expected Result

I'd expect the breadcrumbs to truncate or include the most recent MaxBreadcrumbs breadcrumbs instead of growing a TArray and including the whole thing indefinitely.

Actual Result

All of the breadcrumbs added, including those over the limit, are included in the event or the event is completely rejected for being too large

Any logs or screenshots

[bruno-garcia]

@riot-pmaconi which platform are you targeting? (Windows/iOS/etc)?

Depending on the target OS, we have a different underlying implementation (afaik). So it'll help narrow down what's going on.

[riot-pmaconi]

@bruno-garcia Linux and Windows. The problem areas are probably some combination of these:

https://github.com/getsentry/sentry-unreal/blob/1737695aa0a83637c34ebd499d1e971d24dca23c/plugin-dev/Source/Sentry/Private/Desktop/SentryScopeDesktop.cpp#L36

https://github.com/getsentry/sentry-unreal/blob/1737695aa0a83637c34ebd499d1e971d24dca23c/plugin-dev/Source/Sentry/Private/Desktop/SentryScopeDesktop.cpp#L266

https://github.com/getsentry/sentry-unreal/blob/1737695aa0a83637c34ebd499d1e971d24dca23c/plugin-dev/Source/Sentry/Private/Desktop/SentryScopeDesktop.cpp#L294

[bruno-garcia]

The cap of breadcrumbs should be kept by sentry-native. We can see here that on Android we do the same thing, just add the breadcrumb without consideration of how many are already there:

https://github.com/getsentry/sentry-java/blob/0438c6f3581c589a0772ec049e5bc374588cbeba/sentry-android-ndk/src/main/jni/sentry.c#L154-L216

That goes into sentry-native here: https://github.com/getsentry/sentry-native/blob/7c1d428b4d8a05fcd15c3b191628d370bbe297d3/src/sentry_core.c#L640-L657

And the ringbuffer lives here: https://github.com/getsentry/sentry-native/blob/7c1d428b4d8a05fcd15c3b191628d370bbe297d3/src/sentry_value.c#L646-L678

The event that got dropped had more than 100 (default) breadcrumbs? Or perhaps these crumbs are super long strings that by themselves push through the max size of the payload?

[bruno-garcia]

Do you have a snippet we can use to repro this? (or even a basic AddBreadcrumb with a short string will already repro it?

[riot-pmaconi]

Sure.

USentrySubsystem* SentrySubsystem = GEngine->GetEngineSubsystem<USentrySubsystem>();
for (int i = 0; i < 1000; ++i)
{
    SentrySubsystem->AddBreadcrumbWithParams(FString::FromInt(i), "Warning", FString(), TMap<FString, FString>(), ESentryLevel::Warning);
}
SentrySubsystem->CaptureMessage("TestMessage", ESentryLevel::Error);

[riot-pmaconi]

A little more detail about my last response:

The 1,000 looped breadcrumbs will go through without failing to ingest and all 1000 will show up in the event. If you loop 10,000 times, it will return a 400 and the event will be rejected.

[bruno-garcia]

That's definitely buggy. Thanks for raising it! We'll ship a fix soon