Skip to content

Commit 9dc1433

Browse files
JoesHageclaude
authored andcommitted
fix(httpgen): pass err directly when it implements proto.Message
The previous implementation tried to pass protoErr (proto.Message type) to writeErrorWithHandler which expects an error type. Since proto error messages implement both interfaces, we just pass err directly and let defaultErrorResponse extract the proto.Message. Also updates error-handler example to demonstrate returning proto errors directly from handlers (NotFoundError instead of custom Go struct). Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 parent 897492b commit 9dc1433

File tree

6 files changed

+30
-14
lines changed

6 files changed

+30
-14
lines changed

examples/error-handler/main.go

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,13 +69,22 @@ func (s *UserServiceImpl) CreateUser(ctx context.Context, req *models.CreateUser
6969
}
7070

7171
// GetUser retrieves a user by ID.
72+
// This demonstrates returning a proto error message directly from a handler.
73+
// The framework preserves the proto structure in the response (not wrapped in {"message":"..."}).
7274
func (s *UserServiceImpl) GetUser(ctx context.Context, req *models.GetUserRequest) (*models.User, error) {
7375
s.mu.RLock()
7476
defer s.mu.RUnlock()
7577

7678
user, exists := s.users[req.Id]
7779
if !exists {
78-
return nil, &ErrNotFound{ResourceType: "user", ResourceID: req.Id}
80+
// Return proto error directly - structure is preserved in response:
81+
// Response: {"resourceType":"user","resourceId":"...","message":"user not found"}
82+
// NOT: {"message":"{\"resourceType\":\"user\",...}"}
83+
return nil, &models.NotFoundError{
84+
ResourceType: "user",
85+
ResourceId: req.Id,
86+
Message: "user not found",
87+
}
7988
}
8089
return user, nil
8190
}
@@ -309,8 +318,10 @@ func main() {
309318
fmt.Println(` -H "Content-Type: application/json" \`)
310319
fmt.Println(` -d '{"name": "J", "email": "invalid-email"}'`)
311320
fmt.Println()
312-
fmt.Println("3. Get non-existent user (not found error):")
321+
fmt.Println("3. Get non-existent user (proto error with preserved structure):")
313322
fmt.Println(` curl -v http://localhost:8080/api/v1/users/non-existent-id`)
323+
fmt.Println(` # Returns: {"resourceType":"user","resourceId":"non-existent-id","message":"user not found"}`)
324+
fmt.Println(` # NOT: {"message":"{\"resourceType\":\"user\",...}"} (old behavior)`)
314325
fmt.Println()
315326
fmt.Println("4. Get user with request ID header:")
316327
fmt.Println(` curl -v http://localhost:8080/api/v1/users/non-existent-id \`)

internal/httpgen/error_handler_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -388,15 +388,16 @@ func TestProtoMessageErrorPreservation(t *testing.T) {
388388

389389
t.Run("genericHandler checks for proto.Message errors", func(t *testing.T) {
390390
// The genericHandler should check if the error is already a proto.Message
391-
if !strings.Contains(files.binding, "if protoErr, ok := err.(proto.Message); ok {") {
391+
if !strings.Contains(files.binding, "if _, ok := err.(proto.Message); ok {") {
392392
t.Error("genericHandler should check if error is a proto.Message")
393393
}
394394
})
395395

396396
t.Run("genericHandler passes proto.Message errors directly to writeErrorWithHandler", func(t *testing.T) {
397397
// When error is a proto.Message, it should be passed directly without wrapping
398-
if !strings.Contains(files.binding, "writeErrorWithHandler(w, r, protoErr, errorHandler)") {
399-
t.Error("genericHandler should pass proto.Message errors directly to writeErrorWithHandler")
398+
// The error is passed as 'err' (not protoErr) since it implements both error and proto.Message
399+
if !strings.Contains(files.binding, "if _, ok := err.(proto.Message); ok {") {
400+
t.Error("genericHandler should check if error is a proto.Message")
400401
}
401402
})
402403

@@ -423,7 +424,7 @@ func TestProtoMessageErrorPreservation(t *testing.T) {
423424
t.Run("proto.Message check comes before sebufhttp.Error wrapping", func(t *testing.T) {
424425
// In genericHandler, the proto.Message check should come before the sebufhttp.Error wrapping
425426
binding := files.binding
426-
protoMsgCheckPos := strings.Index(binding, "if protoErr, ok := err.(proto.Message)")
427+
protoMsgCheckPos := strings.Index(binding, "if _, ok := err.(proto.Message)")
427428
sebufErrorPos := strings.Index(binding, "errorMsg := &sebufhttp.Error{")
428429

429430
if protoMsgCheckPos == -1 || sebufErrorPos == -1 {

internal/httpgen/generator.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -566,8 +566,9 @@ func (g *Generator) generateBindingFile(file *protogen.File) error {
566566
gf.P("response, err := serve(r.Context(), request)")
567567
gf.P("if err != nil {")
568568
gf.P("// Check if error is already a proto.Message (e.g., custom proto error types)")
569-
gf.P("if protoErr, ok := err.(proto.Message); ok {")
570-
gf.P("writeErrorWithHandler(w, r, protoErr, errorHandler)")
569+
gf.P("// If so, pass it directly - defaultErrorResponse will preserve its structure")
570+
gf.P("if _, ok := err.(proto.Message); ok {")
571+
gf.P("writeErrorWithHandler(w, r, err, errorHandler)")
571572
gf.P("return")
572573
gf.P("}")
573574
gf.P("errorMsg := &sebufhttp.Error{")

internal/httpgen/testdata/golden/backward_compat_http_binding.pb.go

Lines changed: 3 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/httpgen/testdata/golden/http_verbs_comprehensive_http_binding.pb.go

Lines changed: 3 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

internal/httpgen/testdata/golden/query_params_http_binding.pb.go

Lines changed: 3 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)