Skip to content

refactor(handler): move error conversion to the handler - #331

Merged
justinwon777 merged 2 commits into
mainfrom
justin.won/handler-errors
Oct 9, 2026
Merged

justinwon777 merged 2 commits into
mainfrom
justin.won/handler-errors

Conversation

@justinwon777

Copy link
Copy Markdown
Contributor

The handler now owns the failure log and the wire error conversion. If retErr is not nil, the defer logs " failed" with tangoerrors.Fields and converts the error with toWireError.

Test Plan

unit tests

Issue

The handler now owns the failure log and the wire error conversion. Each handler method uses a named retErr and one defer. If retErr is not nil, the defer logs "<RPC> failed" with tangoerrors.Fields and converts the error with toWireError.

The controller no longer logs failures and no longer converts errors. It returns the classified error. The metrics lifecycle and the repository resolver stay in the controller, so the metrics are the same as on main.

handler.Params gets a Logger field. The server and the integration test pass it.

Test plan:
- make build, make test, make lint
- handler wire error tests run the real controller through the handler

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@justinwon777
justinwon777 requested review from a team as code owners October 9, 2026 02:48
// repository, mirroring the stub of the same name in controller's tests.
type allowAnyRepositoryConfigProvider struct{}

func (allowAnyRepositoryConfigProvider) GetRepositoryConfig(remote string) (config.RepositoryConfig, bool) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

copied from controller/testhelper_test.go but controller version is still needed until handler is fully implemented.

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.

export it? it already depends on controller

@justinwon777
justinwon777 merged commit e42df44 into main Oct 9, 2026
6 of 8 checks passed
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.

2 participants