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

Cronvoy: map Envoy Mobile internal errors to Cronet internal errors #1550

Open
carloseltuerto opened this issue Jun 25, 2021 · 7 comments
Open
Assignees

Comments

@carloseltuerto
Copy link
Contributor

carloseltuerto commented Jun 25, 2021

Cronet exposes 235 internal errors split in 9 categories. Cronet internal error codes are not guaranteed to be stable. However, some of those internal error codes are mapped to the public error code scheme, others are driving some business logic, and couples are used for tests only - having these properly mapped with Envoy Mobile internal error codes will likely avoid regressions. Following error codes should be considered:

Envoy Mobile does expose an internal error code, but will likely require more work:

  • It not always populated, for example, when Envoy Mobile attempts to connect to this URL, "http://127.0.0.1:319", the request does end up as an error, as expected (that socket is purposely invalid). But the error does not provide much details. The internal error code is 0: 'x-internal-error-code', '0', and the error message is an empty String.
  • It does not provide the same error granularity.
@alyssawilk
Copy link
Contributor

@DavidSchinazi when you're back, this feels like a space where I (who know Envoy errors) could collaborate with Renjie (who knows Chrome ones) to do a mapping of sorts?

@DavidSchinazi
Copy link

@alyssawilk that sounds great. Though in this case I'm surprised that Envoy treats a TCP RST as an internal error?

@alyssawilk
Copy link
Contributor

I suspect that's an artifact of the old error pathways that @goaway has mentioned. Upstream reset should be communicated with the UpstreamRemoteReset flag is set and EarlyUpstreamReset/LateUpstreamReset details, which can be converted into the relevant Chrome error

@DavidSchinazi
Copy link

Is there an action item for Cronvoy to set that flag, or will Envoy Mobile do it by default?

@carloseltuerto
Copy link
Contributor Author

carloseltuerto commented Jul 14, 2021

Personally, I would expect Envoy Mobile to provide a fined grained error scheme. ERR_CONNECTION_REFUSED is not the only one here - there are 235 of them.

This is how chromium//net splits the error domain:

     0- 99 System related errors
   100-199 Connection related errors
   200-299 Certificate errors
   300-399 HTTP errors
   400-499 Cache errors
   500-599 Misc errors
   600-699 FTP errors
   700-799 Certificate manager errors
   800-899 DNS resolver errors

@carloseltuerto
Copy link
Contributor Author

But there is somewhere a good news. It is understood that those internal errors are subject to changes, and performing business logic against those is "at your own risks". So we don't need to have the same 235 errors, that's for sure. On the other hand, I guess that for some of these errors, it would be damning if Cronvoy would not return the same error. Here, Envoy Mobile can have its own scheme, and Cronvoy will give them a Cronet "look".

@carloseltuerto
Copy link
Contributor Author

More details were added to the description of the issue.

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

No branches or pull requests

4 participants