Skip to content

Conversation

@harshit078
Copy link

@harshit078 harshit078 commented Jan 13, 2026

Before submitting a pull request, please take a look at our
Contributing guidelines and verify:

  • If you've added code that should be tested, please add tests.
  • Ensure your code lints and the test suite passes (yarn lint) & (yarn test).
  • Link an issue if there is one related to your pull request. If no issue is linked, one will be auto-generated and linked.

Closes #18316

Description

  • This PR adds a new error class ReplayDurationLimitError which handles cases where the replay duration exceeds the allowed limit. The ReplayContainer now throws this error instead of a generic error when the session is too long.
  • this allows the catch block to distinguish between three scenarios 'ratelimit_backoff' | 'send_error' | 'sample_rate';

@harshit078 harshit078 marked this pull request as ready for review January 13, 2026 20:34
@harshit078 harshit078 requested a review from a team as a code owner January 13, 2026 20:34
@chargome chargome self-assigned this Jan 19, 2026
Copy link
Member

@chargome chargome left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @harshit078 thanks for contributing! For the lack of better options in the drop reasons (see https://develop.sentry.dev/sdk/telemetry/client-reports/#envelope-item-payload) I think the most fitting is still send_error in this case.

if (err instanceof RateLimitError) {
dropReason = 'ratelimit_backoff';
} else if (err instanceof ReplayDurationLimitError) {
dropReason = 'sample_rate';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

'sample_rate' would not be the correct drop reason here (this is only used in case an event was dropped because of the configured sample rate).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

got it then if im not wrong then is network_error the appropriate drop reason ?

Copy link
Member

@chargome chargome Jan 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, let's work with ignored for now (We might add a new drop reason in the future!) – please also update the browser integration tests for this

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense, I'll update it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@harshit078 actually we'll introduce a new reason right away so this stays consistant. Mind if I take this PR over? The rest looks good already

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure please go ahead

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if (err instanceof RateLimitError) {
dropReason = 'ratelimit_backoff';
} else if (err instanceof ReplayDurationLimitError) {
dropReason = 'sample_rate';
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect drop reason for duration limit exceeded

Medium Severity

The dropReason for ReplayDurationLimitError is set to 'sample_rate', but this is semantically incorrect. As noted in the PR discussion, 'sample_rate' is specifically meant for events dropped due to the configured sample rate (e.g., sessionSampleRate), not for replays that exceed the maximum duration limit. This will cause misleading analytics where duration-exceeded drops appear as sample-rate drops, making it harder to diagnose why replays are being discarded.

Fix in Cursor Fix in Web

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.

Inaccurate outcome when flushing replays

2 participants