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 EM Errors to Cronet API Errors II (#1594) #2633

Merged
merged 9 commits into from
Nov 2, 2022

Conversation

colibie
Copy link
Contributor

@colibie colibie commented Oct 25, 2022

Description: As a follow up for PR2568, the PR maps the network_disconnected and quic exception errors. The network_changed error has been removed as this is not relevant to envoymobile.
Risk Level: Low
Testing: Included
Docs Changes: n/a
Release Notes: n/a
[Optional Fixes #Issue]
[Optional Deprecated:]

Signed-off-by: Chidera Olibie <colibie@google.com>
The mockurlrequestmock tests are not useful because the different
FailurPhases being tested are not used anywhere in the code.
Also deleted FailurePhase as it's also unneeded

Signed-off-by: Chidera Olibie <colibie@google.com>
This was due to running two engines simultaneously which not
yet fully support(Issue envoyproxy#332)

Signed-off-by: Chidera Olibie <colibie@google.com>
Signed-off-by: Chidera Olibie <colibie@google.com>
@colibie colibie marked this pull request as ready for review October 25, 2022 12:59
@colibie
Copy link
Contributor Author

colibie commented Oct 25, 2022

@Danstahrg for cronet review

@colibie colibie changed the title Cronvoy: Map EM Errors to Cronet API Errors (#1594) Cronvoy: Map EM Errors to Cronet API Errors II (#1594) Oct 25, 2022
Signed-off-by: Chidera Olibie <colibie@google.com>
@colibie colibie requested a review from Danstahrg October 25, 2022 14:29
Signed-off-by: Chidera Olibie <colibie@google.com>
Signed-off-by: Chidera Olibie <colibie@google.com>
Danstahrg
Danstahrg previously approved these changes Oct 27, 2022
Signed-off-by: Chidera Olibie <colibie@google.com>
@colibie
Copy link
Contributor Author

colibie commented Oct 27, 2022

@RyanTheOptimist for EM review and merge

@RyanTheOptimist RyanTheOptimist self-assigned this Nov 1, 2022
Copy link
Contributor

@RyanTheOptimist RyanTheOptimist left a comment

Choose a reason for hiding this comment

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

Looks good. A couple minor comments.

library/java/org/chromium/net/impl/Errors.java Outdated Show resolved Hide resolved
use named constant for quic error also and return
internet_disconnected error before quic_error

Signed-off-by: Chidera Olibie <colibie@google.com>
@RyanTheOptimist RyanTheOptimist merged commit 45c8821 into envoyproxy:main Nov 2, 2022
@colibie colibie deleted the network_disconnected branch November 3, 2022 10:22
jpsim added a commit that referenced this pull request Nov 4, 2022
…builder-function

* origin/main:
  remove the use of deprecated flag (#2658)
  Cronvoy: Map EM Errors to Cronet API Errors II (#1594) (#2633)
  build: revert boring patch (#2651)
  Bump Lyft Support Rotation (#2654)
  fix one issue blocking bumping Envoy (#2649)
  ci: remove Snow from Lyft EM rotation (#2650)
  ci: increasing timeouts (#2653)
  python: Apply Envoy python-yapf formatting (#2648)
  repo: Shellcheck cleanups (#2646)
  repo: Switch `pip_install` -> `pip_parse` (#2647)
  Use safe_malloc instead of new when creating new_envoy_map_entry (#2632)
  python: Pin requirement hashes (#2643)
  Disable flaky TestConfig.StringAccessors (#2642)
  ci: migrate from set-output to GITHUB_OUTPUT (#2625)
  Add a comment to addPlatformFilter (#2634)
  Allow Cronvoy to build with proguard. (#2635)
  Update Envoy (#2630)
  Add support for Platform and Native filters to C++ EngineBuilder (#2626)
  Register getaddrinfo in extension_registry (#2627)
  dns: stop using cares DNS resolver (#2618)

Signed-off-by: JP Simard <jp@jpsim.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.

3 participants