Skip to content

Add catcherror callback for sendData #72

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

Merged
merged 2 commits into from
Apr 4, 2023

Conversation

eliasyishak
Copy link
Contributor

@eliasyishak eliasyishak commented Apr 4, 2023

As pointed out by Danny, the package will cause which ever tool is using it to crash if the client fails to send an event.

This PR adds error handling so that each tool using this package won't crash the tool using it in the case of a http client error (ie. wifi connection drops, client connection was closed before events were sent, etc.)

Google analytics will always return a 2xx status code for events they have received, as such, if a tool using this package would like to confirm that the event sent, they can check the response from analytics.sendData(...) if it is 2xx. However, if the package failed to send the data, this PR will return a status code of 500 indicating that something is wrong with the http client.

@eliasyishak
Copy link
Contributor Author

cc @bwilkerson @DanTup

);
)
.catchError((error) {
return Future<http.Response>.value(http.Response(error.toString(), 500));
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth noting this behaviour in the Dartdoc, just so it's clear to callers that they don't need to handle it, and a response with a 500 code isn't necessarily from the server?

@eliasyishak eliasyishak merged commit 0304fbb into dart-lang:main Apr 4, 2023
@eliasyishak eliasyishak deleted the http-client-error-handling branch April 4, 2023 15:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants