Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5aec2f8848
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1647b00b5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
rust/core/src/bridge.rs
Outdated
| match error { | ||
| Error::Timeout | Error::ConnectionFailed | Error::BridgeError(_) => true, | ||
| #[cfg(any(feature = "bridge", feature = "bridge-wasm"))] | ||
| Error::Http(err) => err.is_connect() || err.is_timeout(), |
There was a problem hiding this comment.
Treat all HTTP polling errors as transient
is_transient_error only retries Error::Http when err.is_connect() or err.is_timeout() is true, but poll_for_status() can also surface other reqwest transport failures (for example while reading/decoding the poll response) via ?; those currently become StatusWrapper::Failed and stop polling permanently. That defeats this change’s goal of recovering from transient bridge communication issues, because a temporary HTTP-layer glitch can still terminate pollUntilCompletion instead of being retried.
Useful? React with 👍 / 👎.
Motivation
While testing IDKit on Android I saw some transient errors that stopped the bridge polling, in my case there was an obscure
DNS errorthat happened when my android device entered foreground.Changes
Introduced a new error variant for android and kotlin
Error::Transientthat is meant to be retried by the SDK user.pollUntilCompletionthey don't need to change anything, this is an improvemntpollStatusOncethis is a new error they can handle, this is breaking if you have an exhausting switch/whenWhy a new error variant? The current
Failedvariant indicated the polling is stopped and the error should be handled to make a decision, I believe keeping this separate helps.