Skip to content

Cronvoy: Map EM Errors to Cronet API Errors (#1594)#2568

Merged
RyanTheOptimist merged 17 commits intoenvoyproxy:mainfrom
colibie:cronvoy_errors
Oct 20, 2022
Merged

Cronvoy: Map EM Errors to Cronet API Errors (#1594)#2568
RyanTheOptimist merged 17 commits intoenvoyproxy:mainfrom
colibie:cronvoy_errors

Conversation

@colibie
Copy link
Contributor

@colibie colibie commented Sep 21, 2022

Description: Converts EM's responseFlags to Cronet's API Errors
Additional Description: Note that this change does not include ERR_NETWORK_CHANGED, ERR_INTERNET_DISCONNECT and QUIC_PROTOCOL_FAILED as they aren't implemented yet
Risk Level: Low
Testing: Test included
Docs Changes: n/a
Release Notes: n/a
[Optional Fixes #Issue]
[Optional Deprecated:]

@colibie colibie marked this pull request as draft September 21, 2022 23:59
Signed-off-by: Chidera Olibie <colibie@google.com>
@colibie
Copy link
Contributor Author

colibie commented Sep 22, 2022

@Danstahrg for cronet review

@colibie colibie marked this pull request as ready for review September 22, 2022 09:14
Copy link

@Danstahrg Danstahrg left a comment

Choose a reason for hiding this comment

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

General recommendation: Consider splitting the tangential refactors to a separate change, it makes the pull request more self explanatory.

Signed-off-by: Chidera Olibie <colibie@google.com>
Signed-off-by: Chidera Olibie <colibie@google.com>
Signed-off-by: Chidera Olibie <colibie@google.com>
Signed-off-by: Chidera Olibie <colibie@google.com>
Signed-off-by: Chidera Olibie <colibie@google.com>
Signed-off-by: Chidera Olibie <colibie@google.com>
Signed-off-by: Chidera Olibie <colibie@google.com>
Signed-off-by: Chidera Olibie <colibie@google.com>
@colibie colibie requested a review from Danstahrg October 7, 2022 12:03
Danstahrg
Danstahrg previously approved these changes Oct 10, 2022
@colibie
Copy link
Contributor Author

colibie commented Oct 10, 2022

@Augustyniak could you please review? This resolves the TODO you added in PR2565

@alyssawilk for the c++

Augustyniak
Augustyniak previously approved these changes Oct 10, 2022
Copy link
Contributor

@Augustyniak Augustyniak left a comment

Choose a reason for hiding this comment

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

+1 for addressing TODO, thank you!

Signed-off-by: Chidera Olibie <colibie@google.com>
Signed-off-by: Chidera Olibie <colibie@google.com>
@alyssawilk
Copy link
Contributor

-> Ryan for C++ as I'm largely out this week

Signed-off-by: Chidera Olibie <colibie@google.com>
@colibie colibie dismissed stale reviews from Augustyniak and Danstahrg via 445d3ce October 18, 2022 14:38
Signed-off-by: Chidera <colibie@google.com>
Moved the testread filter to test/integration/filters.
This is where @envoy's own test-only filters are.

Signed-off-by: Chidera Olibie <colibie@google.com>
@RyanTheOptimist RyanTheOptimist merged commit a9aafa2 into envoyproxy:main Oct 20, 2022
colibie added a commit to colibie/envoy-mobile that referenced this pull request Oct 22, 2022
…roxy#2568)

Description: Converts EM's responseFlags to Cronet's API Errors
Additional Description: Note that this change does not include ERR_NETWORK_CHANGED, ERR_INTERNET_DISCONNECT and QUIC_PROTOCOL_FAILED as they aren't implemented yet
Risk Level: Low
Testing: Test included
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Chidera Olibie <colibie@google.com>

Signed-off-by: Chidera <colibie@google.com>
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.

5 participants