Skip to content

refactor(model): collapse coded error types into a single CodedErr - #896

Open
areebahmeddd wants to merge 5 commits into
beckn:mainfrom
areebahmeddd:refactor/collapse-coded-error-types
Open

refactor(model): collapse coded error types into a single CodedErr#896
areebahmeddd wants to merge 5 commits into
beckn:mainfrom
areebahmeddd:refactor/collapse-coded-error-types

Conversation

@areebahmeddd

Copy link
Copy Markdown
Contributor

Summary

  • replaces BadReqErr, SignValidationErr, and NotFoundErr with a single model.CodedErr, reducing duplicated error types while preserving existing response behavior
  • introduces a more flexible error construction API with explicit status codes, making it easier for plugins to return different HTTP statuses without relying on concrete error types
  • keeps wire compatibility unchanged, updates all existing call sites, and extends tests to verify both error codes and HTTP status mappings

Fixes #888

@areebahmeddd
areebahmeddd requested a review from nirmay as a code owner August 1, 2026 20:01
Copilot AI review requested due to automatic review settings August 1, 2026 20:01

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors Beckn step error handling by collapsing multiple coded error wrapper types (BadReqErr, SignValidationErr, NotFoundErr) into a single model.CodedErr that carries both the taxonomy code and an explicit HTTP status, and updates handler dispatch + call sites to preserve existing on-wire behavior while enabling more flexible status selection.

Changes:

  • Introduces model.CodedErr (+ NewCodedErr, NewBadReqErr, NewSignValidationErr, NewNotFoundErr) and updates nackBecknError to route by CodedErr.HTTPStatus().
  • Updates plugins/core to construct CodedErr via the new constructors and adjusts comments/docs accordingly.
  • Extends/updates tests to assert both taxonomy codes and the intended HTTP status mappings.

Reviewed changes

Copilot reviewed 32 out of 32 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
pkg/testutil/becknerr.go Updates shared test assertion helper to assert *model.CodedErr and HTTP 400 mapping.
pkg/plugin/implementation/vcvalidator/vcvalidator.go Switches vcvalidator NACK construction to new error constructors.
pkg/plugin/implementation/vcvalidator/vcvalidator_test.go Updates vcvalidator tests to assert CodedErr and HTTP status values.
pkg/plugin/implementation/vcvalidator/README.md Updates documentation to reflect CodedErr constructors/statuses.
pkg/plugin/implementation/simplekeymanager/simplekeymanager.go Replaces SignValidation error construction with new constructors.
pkg/plugin/implementation/simplekeymanager/simplekeymanager_test.go Updates tests to assert *model.CodedErr instead of removed types.
pkg/plugin/implementation/signvalidator/signvalidator.go Updates signvalidator to return CodedErr via new constructors.
pkg/plugin/implementation/signvalidator/signvalidator_test.go Updates signvalidator tests to assert CodedErr and HTTP 401 mapping.
pkg/plugin/implementation/schemavalidator/schemavalidator.go Updates bad-request construction to new NewBadReqErr(code, err) signature.
pkg/plugin/implementation/schemav2validator/schemav2validator.go Updates bad-request construction to new NewBadReqErr(code, err) signature.
pkg/plugin/implementation/router/router.go Updates bad-request coded error construction for invalid routing targets.
pkg/plugin/implementation/reqmapper/reqmapper.go Updates bad-request coded error construction for missing/invalid action.
pkg/plugin/implementation/registry/registry.go Updates bad-request construction for marshal failures to new signature.
pkg/plugin/implementation/publisher/publisher.go Updates bad-request coded error construction for downstream publish failures.
pkg/plugin/implementation/publisher/publisher_test.go Updates test to assert *model.CodedErr for publish failure classification.
pkg/plugin/implementation/opapolicychecker/enforcer.go Updates policy checker to return CodedErr via NewBadReqErr.
pkg/plugin/implementation/opapolicychecker/enforcer_test.go Updates OPA tests to assert *model.CodedErr instead of removed types.
pkg/plugin/implementation/keymanager/keymanager.go Replaces SignValidation error construction with new constructors.
pkg/plugin/implementation/keymanager/keymanager_test.go Updates tests to assert *model.CodedErr instead of removed types.
pkg/plugin/implementation/encrypter/encrypter.go Updates error construction to use NewBadReqErr(code, err) signature.
pkg/plugin/implementation/decrypter/decrypter.go Updates error construction to use NewBadReqErr(code, err) signature.
pkg/model/model.go Updates WrapExtractContextErr to return *CodedErr and use new constructor.
pkg/model/error.go Introduces CodedErr + constructors and updates BecknErrorer guidance.
pkg/model/error_test.go Updates/extends tests for new CodedErr behavior and status mapping.
core/module/module_test.go Updates policy checker mock to use new NewBadReqErr(code, err) signature.
core/module/handler/step.go Updates bad-request/sign-validation error construction to new signatures.
core/module/handler/step_test.go Updates tests to assert *model.CodedErr and status for schema/sign validation.
core/module/handler/stdHandler.go Updates body-read failure to new NewBadReqErr(code, err) signature.
core/module/handler/stdHandler_test.go Updates failing step mock to new NewBadReqErr(code, err) signature.
core/module/handler/responsestep.go Refactors NACK dispatch to use *model.CodedErr + HTTPStatus() instead of type-switching old error types.
core/module/handler/responsestep_test.go Updates response-step tests to new constructors and expected status mappings.
cmd/adapter/main_test.go Updates adapter test policy checker stub to new NewBadReqErr(code, err) signature.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/model/error.go Outdated
Comment thread pkg/model/error.go
Comment thread pkg/plugin/implementation/vcvalidator/vcvalidator_test.go Outdated
@areebahmeddd

Copy link
Copy Markdown
Contributor Author

Deliberately skipped

One behavior difference

  • the old dispatch selected by type priority (SignValidationErr > BadReqErr > NotFoundErr) regardless of wrapping depth. the new dispatch uses the outermost CodedErr
  • this only matters if an error chain contains multiple CodedErrs. traced every path and none do today, so the behavior is currently unreachable

Loose thread

  • NewNotFoundErr has no production callers, only tests. it may be dead code

Copilot AI review requested due to automatic review settings August 1, 2026 20:19

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 32 out of 32 changed files in this pull request and generated no new comments.

Suppressed comments (1)

pkg/model/error.go:232

  • CodedErr.Error() is documented as avoiding panics while building a NACK, but it can still panic if the receiver is nil (typed-nil *CodedErr stored in an error interface) or if the wrapped cause is a typed-nil error whose Error() dereferences a nil pointer. Making Error() nil-safe and panic-safe here prevents a request from crashing the handler during NACK rendering.
func (e *CodedErr) Error() string {
	if e.error == nil {
		return ""
	}
	return e.error.Error()

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.

Collapse BadReqErr/SignValidationErr/NotFoundErr into one generic coded-error type with explicit HTTP status

2 participants