fiber: fix extra brace in optional pass-through cookie binding#2443
Conversation
The cookie `IsPassThrough` branch in the fiber v2 middleware template emitted `}&cookie` instead of `&cookie` when `HasOptionalPointer` is true, producing a compile error in any generated server with an optional pass-through cookie parameter. No committed fixture exercises such a parameter, which is how it went unnoticed.
Greptile SummaryThis PR fixes a one-character typo in the fiber v2 middleware template where the
Confidence Score: 4/5The template change is minimal, targeted, and correct; it unblocks any downstream user who generates a fiber server with an optional pass-through cookie parameter. The fix is a one-character removal with no risk of unintended side-effects. The only open gap is the absence of a regression test — the bug was originally invisible for this exact reason, and adding a roundtrip fixture would close that gap before a future refactor reintroduces it. No files require special attention beyond the optional addition of a roundtrip test fixture for optional pass-through cookie parameters.
|
| Filename | Overview |
|---|---|
| pkg/codegen/templates/fiber/fiber-middleware.tmpl | Single-character template fix: removes an extra literal } from the IsPassThrough + HasOptionalPointer cookie branch, eliminating a compile error in any generated fiber server that uses an optional pass-through cookie parameter. |
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
pkg/codegen/templates/fiber/fiber-middleware.tmpl:148
**Missing regression test for the fixed path**
The PR description acknowledges that no fixture exercises an optional pass-through cookie parameter in the fiber backend, which is exactly why the bug went unnoticed. The fix prevents a future regression, but without a test the same path can silently break again.
Per the project's test placement guidelines, this belongs in `internal/test/parameters/roundtrip/parameters.yaml` — the existing `/cookie` endpoint (line 522) could be extended with an optional `content`-typed (pass-through) cookie parameter, and the fiber generated file regenerated. A comment like `# From issue-NNNN: optional pass-through cookie in fiber emitted extra brace` preserves traceability.
Reviews (1): Last reviewed commit: "fiber: fix extra brace in optional pass-..." | Re-trigger Greptile
|
|
||
| {{- if .IsPassThrough}} | ||
| params.{{.GoName}} = {{if .HasOptionalPointer}}}&{{end}}cookie | ||
| params.{{.GoName}} = {{if .HasOptionalPointer}}&{{end}}cookie |
There was a problem hiding this comment.
The PR description acknowledges that no fixture exercises an optional pass-through cookie parameter in the fiber backend, which is exactly why the bug went unnoticed. The fix prevents a future regression, but without a test the same path can silently break again.
Per the project's test placement guidelines, this belongs in internal/test/parameters/roundtrip/parameters.yaml — the existing /cookie endpoint (line 522) could be extended with an optional content-typed (pass-through) cookie parameter, and the fiber generated file regenerated. A comment like # From issue-NNNN: optional pass-through cookie in fiber emitted extra brace preserves traceability.
Prompt To Fix With AI
This is a comment left during a code review.
Path: pkg/codegen/templates/fiber/fiber-middleware.tmpl
Line: 148
Comment:
**Missing regression test for the fixed path**
The PR description acknowledges that no fixture exercises an optional pass-through cookie parameter in the fiber backend, which is exactly why the bug went unnoticed. The fix prevents a future regression, but without a test the same path can silently break again.
Per the project's test placement guidelines, this belongs in `internal/test/parameters/roundtrip/parameters.yaml` — the existing `/cookie` endpoint (line 522) could be extended with an optional `content`-typed (pass-through) cookie parameter, and the fiber generated file regenerated. A comment like `# From issue-NNNN: optional pass-through cookie in fiber emitted extra brace` preserves traceability.
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
The cookie
IsPassThroughbranch in the fiber v2 middleware template emitted}&cookieinstead of&cookiewhenHasOptionalPointeris true, producing a compile error in any generated server with an optional pass-through cookie parameter. No committed fixture exercises such a parameter, which is how it went unnoticed.