Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Investigate SDK back pressure issues #2360

Closed
denrase opened this issue Oct 15, 2024 · 4 comments
Closed

Investigate SDK back pressure issues #2360

denrase opened this issue Oct 15, 2024 · 4 comments
Assignees

Comments

@denrase
Copy link
Collaborator

denrase commented Oct 15, 2024

Description

A user reported that calling sentry capture methods in fast succession is leading to issues, especially with screenshot integration.

Investigate if calling the SDK mehods in tight loops can be improved. One idea was to introduce a debounce for screenshots.

The minimal repro steps I got are attached, but this is a heavily contrived example aimed at easily reproducing the issue. In practice this would look like 30 API requests in short succession that all fail due to no internet connectivity or where all endpoints return an error code, where Sentry then creates exceptions in short succession.

Image

@buenaflor
Copy link
Contributor

fyi, the real example could lead up to 75 api calls due to heavy retries (for example with bad internet connection)

@denrase denrase moved this from Needs Discussion to Needs Investigation in Mobile & Cross Platform SDK Oct 21, 2024
@denrase
Copy link
Collaborator Author

denrase commented Oct 21, 2024

Looking at the code, we do drop the envelopes due to back pressure, but this is probably happening too late, as we only do this before calling transport.

() => _options.transport.send(envelope),

Image

We could either move this to an earlier place in the processing queue, or introduce additional measured, like the mentioned debounce for the screenshot widget. The latter is probably a good addition anyway.

@denrase denrase moved this from Needs Investigation to Needs Review in Mobile & Cross Platform SDK Oct 22, 2024
@denrase
Copy link
Collaborator Author

denrase commented Oct 22, 2024

@buenaflor Added a PR for widget debounce. Do you think this will resolve this issue or should we introduce additional measures?

@buenaflor
Copy link
Contributor

We could either move this to an earlier place in the processing queue

I think this might also be a good solution so we don't trigger all the event processors and thus decrease overhead, right?

Added a PR for widget debounce

👍 maybe might be good to check out with the flutter profiler to see if there is any other event processor or similar that is creating overhead when calling captureException in a tight loop

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

No branches or pull requests

2 participants