Skip to content

fix(rpc-service): Consider all Infura HTTP errors as service failures except 400 and 429#9123

Merged
FrederikBolding merged 8 commits into
mainfrom
fb/treat-more-errors-as-service-failures
Jun 17, 2026
Merged

fix(rpc-service): Consider all Infura HTTP errors as service failures except 400 and 429#9123
FrederikBolding merged 8 commits into
mainfrom
fb/treat-more-errors-as-service-failures

Conversation

@FrederikBolding

@FrederikBolding FrederikBolding commented Jun 15, 2026

Copy link
Copy Markdown
Member

Explanation

Consider all HTTP errors thrown by RpcService when using Infura as service policy failures except if the HTTP status code is 400 or 429. Previously we would only consider 5xx HTTP status codes as failures that could trip the circuit.

References

https://consensyssoftware.atlassian.net/browse/WPC-1111

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

@FrederikBolding FrederikBolding changed the title fix: Consider all HTTP errors as service failures except 429 fix(rpc-service): Consider all HTTP errors as service failures except 429 Jun 15, 2026
@FrederikBolding FrederikBolding marked this pull request as ready for review June 15, 2026 12:48
@FrederikBolding FrederikBolding requested review from a team as code owners June 15, 2026 12:48
@FrederikBolding FrederikBolding changed the title fix(rpc-service): Consider all HTTP errors as service failures except 429 fix(rpc-service): Consider all Infura HTTP errors as service failures except 400 and 429 Jun 16, 2026
Comment thread packages/network-controller/src/rpc-service/rpc-service.ts Fixed

@cryptodev-2s cryptodev-2s left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The changes look good to me. I think they just warrant adding some unit tests to cover the new cases.

While testing I added some tests locally and now I opened this PR to make sure we don’t break anything: #9169

Let me know what you think.

@FrederikBolding

Copy link
Copy Markdown
Member Author

The changes look good to me. I think they just warrant adding some unit tests to cover the new cases.

While testing I added some tests locally and now I opened this PR to make sure we don’t break anything: #9169

Let me know what you think.

Those LGTM!

…on (#9169)

Follow up to #9123 to add the missing test coverage and fix a stale
comment. Targets that PR's branch so it merges in before #9123 goes to
main.

## What this adds

`packages/controller-utils` (`create-service-policy.test.ts`):
- Custom `isServiceFailure` opens the circuit when the predicate treats
the error as a failure, and never opens it when it does not.
- The predicate is called with the raw error thrown by the service.
- The default predicate still applies when the option is omitted (breaks
on `httpStatus >= 500`, not on `< 500`).

`packages/network-controller` (`rpc-service.test.ts`):
- On an Infura endpoint, 400 and 429 do not break the circuit, while 401
and 500 do.
- A non Infura endpoint with the same config does not break on 401,
which proves the Infura predicate is what changes the behavior.
@FrederikBolding FrederikBolding added this pull request to the merge queue Jun 17, 2026
Merged via the queue into main with commit 5ed6ac5 Jun 17, 2026
379 checks passed
@FrederikBolding FrederikBolding deleted the fb/treat-more-errors-as-service-failures branch June 17, 2026 14:00
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.

4 participants