Skip to content

Strictgin error handler#1600

Open
actatum wants to merge 10 commits intooapi-codegen:mainfrom
actatum:strictgin-error-handler
Open

Strictgin error handler#1600
actatum wants to merge 10 commits intooapi-codegen:mainfrom
actatum:strictgin-error-handler

Conversation

@actatum
Copy link
Copy Markdown

@actatum actatum commented May 6, 2024

Strict gin server doesn't currently use an error handler function. This adds the error handler to the strict gin handler

@jamietanna jamietanna modified the milestones: v2.2.0, v2.3.0 Jun 1, 2024
@jamietanna jamietanna modified the milestones: v2.4.0, v2.5.0 Sep 20, 2024
@jamietanna jamietanna modified the milestones: v2.5.0, v2.6.0 May 11, 2025
Resolve conflict in strict-gin.tmpl: combine error handler function
(from PR) with optional body handling (from upstream).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@mromaszewicz
Copy link
Copy Markdown
Member

Ok, so I know what to do here. I'm going to have three categories of error callbacks.

  1. RequestErrorHandlerFunc which is invoked when there is an error unmarshaling a request, this will default to HTTP/400
  2. HandlerErrorFunc, which is probably the one most interesting to users. This is called if the handler code returns an error.
  3. ResponseErrorHandlerFunc, which gets called if the response serialization vistor encounters an error.

The problem with having a single callback, is that you have no idea where it was called from, so you don't have good context on how to handle that error. With these three error handler types, I think we can retain current behavior (which this PR breaks without @jamietanna 's changes) and allow for much easier error customization.

…ict gin template

Split the single ErrorHandlerFunc into three distinct callbacks to give
callers control over error responses by category:

- RequestErrorHandlerFunc: request parsing/binding errors (default 400)
- HandlerErrorFunc: application handler errors (default 500)
- ResponseErrorHandlerFunc: response serialization errors (default 500)

This also fixes two bugs in the original template:
- Missing multipart boundary now passes http.ErrMissingBoundary instead
  of a nil error
- Unexpected response type now passes a descriptive error instead of nil

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@mromaszewicz mromaszewicz requested a review from a team as a code owner March 5, 2026 00:40
@mromaszewicz
Copy link
Copy Markdown
Member

@greptileai

@greptile-apps
Copy link
Copy Markdown

greptile-apps bot commented Mar 5, 2026

Greptile Summary

This PR adds configurable error handling to the strict Gin server by introducing a StrictGinServerOptions struct with three handler functions (RequestErrorHandlerFunc, HandlerErrorFunc, ResponseErrorHandlerFunc), a new NewStrictHandlerWithOptions constructor, and safe defaults wired through NewStrictHandler. The generated code is correctly updated across all test fixtures.

Key changes:

  • StrictGinServerOptions struct added to the Gin strict template with all three error-path callback fields.
  • NewStrictHandler now initialises the struct with sensible defaults (400 for parse errors, 500 for handler/response errors) instead of writing inline.
  • NewStrictHandlerWithOptions validates all three function fields for nil and falls back to the same defaults, preventing the nil-pointer dereference panics that were possible before.
  • All error paths in the generated handler body (RequestErrorHandlerFunc, HandlerErrorFunc, ResponseErrorHandlerFunc) are correctly threaded through the options struct.
  • The if / else if / else if response-dispatch chain remains unchanged and is structurally sound — only one branch fires per invocation.

Confidence Score: 4/5

  • PR is safe to merge; the implementation is correct and the nil-pointer concern from prior review has been resolved.
  • The core logic is sound: nil checks are present for all three handler functions in NewStrictHandlerWithOptions, defaults are consistent with the pre-existing inline behaviour, and return statements are in place after every early-exit error path. Score is 4 rather than 5 because the default handlers surface err.Error() verbatim in the JSON response body, which can leak internal details in production deployments — users need to supply custom handlers via NewStrictHandlerWithOptions to mitigate this.
  • pkg/codegen/templates/strict/strict-gin.tmpl — review the default error response format if deploying to production.

Important Files Changed

Filename Overview
pkg/codegen/templates/strict/strict-gin.tmpl Adds StrictGinServerOptions struct and NewStrictHandlerWithOptions constructor, wiring all error paths through configurable handler functions with nil-safe defaults. Logic is correct, nil-checks are present, and the if-else chain correctly prevents double-writes.

Last reviewed commit: d2d8b62

Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
@mromaszewicz
Copy link
Copy Markdown
Member

@greptileai

I accepted a Greptile code fix, but this affected generated code.
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