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

fix: adding an error handler to the segment client #5505

Merged
merged 1 commit into from
Dec 5, 2023

Conversation

mkhq
Copy link
Contributor

@mkhq mkhq commented Dec 4, 2023

What this PR does / why we need it:

The segment client has a new API in the updated version. It calls an errorHandler function when there are network level failures such as DNS lookup errors. This change logs the error description on the debug level instead of a full error output on the console.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

@mkhq mkhq requested a review from a team December 4, 2023 14:09
@10ko
Copy link
Member

10ko commented Dec 4, 2023

I wonder if we should implement a retry? Or are we ok if we loose some data?

@mkhq
Copy link
Contributor Author

mkhq commented Dec 4, 2023

I wonder if we should implement a retry? Or are we ok if we loose some data?

There is a retry mechanism in the flush method. This is a band-aid on the existing library, we should replace this with the newer segment client.

@10ko
Copy link
Member

10ko commented Dec 4, 2023

aha, gotcha. 👍

@mkhq mkhq enabled auto-merge December 4, 2023 15:18
@mkhq mkhq added this pull request to the merge queue Dec 5, 2023
Merged via the queue into main with commit 540498c Dec 5, 2023
45 checks passed
@mkhq mkhq deleted the fix-analytics-crash-network branch December 5, 2023 10:03
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.

3 participants