-
Notifications
You must be signed in to change notification settings - Fork 8
Add Connect mux with VProto error handling and improve error hooks #504
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
Conversation
…c.go, http.go, and vproto.go
…and improved readability
…ruct in gormx/database.go
There was a problem hiding this comment.
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 adds Connect mux functionality with VProto error handling and improves error handling hooks throughout the codebase. The changes introduce a new Connect multiplexer that can handle both VProto and Connect error formats while standardizing error message formatting.
- Adds new Connect mux implementation with VProto error handling support
- Improves error handling hooks and simplifies error writer configuration
- Standardizes error message capitalization and removes unused imports
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| connectx/mux.go | New Connect multiplexer with VProto error handling and gRPC protocol rejection |
| statusx/vproto.go | Updates VProto error handling to support Connect errors and simplifies configuration |
| statusx/http.go | Improves HTTP error writer with better hook handling and error message formatting |
| normalize/grpc.go | Replaces statusx with standard errors package and updates error messages |
| gormx/database.go | Removes inject tag from tracing configuration |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <[email protected]>
| // Why not use statusx.TranslateError? Just to avoid affecting the original prottp releated logic. | ||
| err, _ := TranslateStatusErrorOnly(ctx, input.Conf.I18N, input.Err) | ||
| written := WriteConnectErrorOnly(errWriter, input.W, input.R, err) | ||
| return &HTTPWriteErrorOutput{Written: written}, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Middleware Chain Breakage and Unnecessary Allocations
The VProtoHTTPWriteErrorHook now directly writes a Connect error when EnsureConnectError(ctx) is true, bypassing the next handler and breaking the middleware chain. Additionally, connect.NewErrorWriter() is instantiated for each error, leading to unnecessary allocations and potential performance impact.
No description provided.