docs: clarify diagnostic panic for nil UsedCodes in TOTPHandler#342
Merged
veverkap merged 2 commits intoMay 23, 2026
Merged
Conversation
The isReplay and recordUsed helpers now panic with an explicit message 'TOTPHandler misconfigured: UsedCodes is nil; call Validate() at server startup' instead of producing a cryptic nil pointer dereference. Update the warning admonition to document the exact panic message and explain why this is intentional (recover-based middleware can swallow silent panics). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the TOTPHandler documentation to accurately describe the intentional diagnostic panic that occurs when UsedCodes is nil at runtime (matching the current handler/totp.go behavior), including the exact panic message and the rationale for it in recover()-based deployments.
Changes:
- Updated the
UsedCodes is requiredwarning indocs/handler/totp.mdto quote the exact panic message developers will see. - Added explanation of why the diagnostic panic is intentional (clear root cause vs. nil deref under recovery middleware).
- Preserved existing guidance to initialize
UsedCodesand callValidate()at server startup.
Show a summary per file
| File | Description |
|---|---|
| docs/handler/totp.md | Clarifies nil UsedCodes behavior by documenting the exact diagnostic panic message and rationale. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 1/1 changed files
- Comments generated: 0
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Updates
docs/handler/totp.mdto accurately reflect the diagnostic panic behavior restored in #339.What changed in code (#339)
TOTPHandler.isReplayandTOTPHandler.recordUsednow panic with an explicit, actionable message:rather than producing a silent nil pointer dereference. This matters in production deployments that use
recover()-based middleware, which can swallow cryptic panics and make the root cause impossible to diagnose.What this PR changes
!!! warning "UsedCodes is required"admonition indocs/handler/totp.mdto:recover()-based middleware context)UsedCodesand callValidateat startupChecklist
UsedCodesnil panic guards in TOTP replay helpers #339Add this agentic workflows to your repo
To install this agentic workflow, run
Greptile Summary
This documentation-only PR updates the
!!! warningadmonition indocs/handler/totp.mdto accurately reflect the explicit diagnostic panic introduced in #339 for nilUsedCodes.TOTPHandler misconfigured: UsedCodes is nil; call Validate() at server startup) matcheshandler/totp.golines 77 and 85 verbatim.Confidence Score: 5/5
Documentation-only change; no executable code is modified.
The panic message quoted in the admonition matches the source exactly (handler/totp.go lines 77 and 85), the scope (Enroll and Verify) is correct, and the startup guidance is intact. Nothing in the change can affect runtime behavior.
No files require special attention.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Server startup] --> B[Initialize TOTPHandler\nwith UsedCodes set] B --> C[Call h.Validate] C -->|error| D[log.Fatal — misconfiguration\nsurfaces immediately] C -->|nil| E[Register routes] E --> F{Enroll or Verify\nrequest arrives} F --> G[isReplay / recordUsed\nchecks UsedCodes] G -->|UsedCodes == nil| H["panic: TOTPHandler misconfigured:\nUsedCodes is nil; call Validate()\nat server startup"] G -->|UsedCodes != nil| I[Normal handler flow]Reviews (1): Last reviewed commit: "Merge branch 'main' into docs/update-tot..." | Re-trigger Greptile