Skip to content

refactor(errors): remove SubCategoryUserError — charge normal CU for …#2273

Merged
nimrod-teich merged 1 commit intomainfrom
refactor/remove-user-error-subcategory
Apr 16, 2026
Merged

refactor(errors): remove SubCategoryUserError — charge normal CU for …#2273
nimrod-teich merged 1 commit intomainfrom
refactor/remove-user-error-subcategory

Conversation

@NadavLevi
Copy link
Copy Markdown
Collaborator

…user-input errors

User-input errors (Layer D: invalid params, bad hex, parse errors) were previously zero-CU like unsupported-method errors. This was wrong because user-error responses are NOT cached — the provider does real work on every call and should be compensated. Only unsupported-method responses (which ARE cached) justify the zero-CU carve-out.

  • Remove SubCategoryUserError, IsUserError(), IsNonRetryableUserFacing()
  • Remove IsUserInputError() classifier function
  • Remove IsUserError field from RelayResult and NodeErrorClassification
  • Remove IsNonRetryableUserFacingErrorType() (now redundant)
  • Layer D codes keep Retryable=false (no retry) but SubCategory=None (normal CU)
  • Zero-CU gate simplified: only IsUnsupportedMethod triggers it
  • HasNonRetryableUserFacingErrors now uses IsNonRetryable flag directly

Description

Closes: #XXXX


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • read the contribution guide
  • included the correct type prefix in the PR title, you can find examples of the prefixes below:
  • confirmed ! in the type prefix if API or client breaking change
  • targeted the main branch
  • provided a link to the relevant issue or specification
  • reviewed "Files changed" and left comments if necessary
  • included the necessary unit and integration tests
  • updated the relevant documentation or specification, including comments for documenting Go code
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic, API design and naming, documentation is accurate, tests and test coverage

@NadavLevi NadavLevi requested a review from AnnaR-prog April 15, 2026 06:58
@NadavLevi NadavLevi force-pushed the refactor/remove-user-error-subcategory branch 2 times, most recently from ed683ed to 376c082 Compare April 15, 2026 07:06
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 15, 2026

Codecov Report

❌ Patch coverage is 0% with 4 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
protocol/relaycore/relay_processor.go 0.00% 1 Missing and 1 partial ⚠️
protocol/rpcconsumer/rpcconsumer_server.go 0.00% 2 Missing ⚠️
Flag Coverage Δ
consensus 8.96% <ø> (-0.02%) ⬇️
protocol 35.20% <0.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
protocol/chainlib/node_error_handler.go 67.41% <ø> (+1.48%) ⬆️
protocol/common/endpoints.go 0.00% <ø> (ø)
protocol/common/error_classifier.go 71.91% <ø> (-5.94%) ⬇️
protocol/common/error_registry.go 80.51% <ø> (-2.61%) ⬇️
...otocol/rpcconsumer/consumer_relay_state_machine.go 76.58% <ø> (ø)
protocol/relaycore/relay_processor.go 56.78% <0.00%> (-0.24%) ⬇️
protocol/rpcconsumer/rpcconsumer_server.go 32.31% <0.00%> (+0.21%) ⬆️

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 15, 2026

Test Results

0 tests  ±0   0 ✅ ±0   0s ⏱️ ±0s
0 suites ±0   0 💤 ±0 
7 files   ±0   0 ❌ ±0 

Results for commit 56a1e29. ± Comparison against base commit 2d92617.

♻️ This comment has been updated with latest results.

…user-input errors

User-input errors (Layer D: invalid params, bad hex, parse errors) were
previously zero-CU like unsupported-method errors. This was wrong because
user-error responses are NOT cached — the provider does real work on every
call and should be compensated. Only unsupported-method responses (which
ARE cached) justify the zero-CU carve-out.

- Remove SubCategoryUserError, IsUserError(), IsNonRetryableUserFacing()
- Remove IsUserInputError() classifier function
- Remove IsUserError field from RelayResult and NodeErrorClassification
- Remove IsNonRetryableUserFacingErrorType() (now redundant)
- Layer D codes keep Retryable=false (no retry) but SubCategory=None (normal CU)
- Zero-CU gate simplified: only IsUnsupportedMethod triggers it
- HasNonRetryableUserFacingErrors now uses IsNonRetryable flag directly

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@NadavLevi NadavLevi force-pushed the refactor/remove-user-error-subcategory branch from 376c082 to 56a1e29 Compare April 15, 2026 16:08
@nimrod-teich nimrod-teich merged commit 21fbf75 into main Apr 16, 2026
36 of 38 checks passed
@nimrod-teich nimrod-teich deleted the refactor/remove-user-error-subcategory branch April 16, 2026 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants