Skip to content

fix: hide panic details from clients#86

Merged
raeperd merged 1 commit into
mainfrom
fix/hide-panic-details
May 6, 2026
Merged

fix: hide panic details from clients#86
raeperd merged 1 commit into
mainfrom
fix/hide-panic-details

Conversation

@raeperd
Copy link
Copy Markdown
Owner

@raeperd raeperd commented May 6, 2026

Summary

  • Return a generic internal server error response for recovered panics
  • Keep the original panic value and stack trace in logs
  • Add test coverage to ensure panic details are not exposed to clients

Test

  • go test -race ./...
  • golangci-lint run

Summary by CodeRabbit

Release Notes

  • Bug Fixes

    • Improved server error handling to return a generic error message instead of detailed panic information when an error occurs.
  • Tests

    • Enhanced test coverage for the recovery middleware to verify proper error response messages and panic logging behavior.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 6, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 638a0837-0dcb-43d5-b72b-b582004fc8bd

📥 Commits

Reviewing files that changed from the base of the PR and between 93ac24b and ee18d8c.

📒 Files selected for processing (2)
  • main.go
  • main_test.go

📝 Walkthrough

Walkthrough

The panic-recovery middleware now returns a generic "internal server error" message instead of echoing the detailed panic error to clients. Tests are updated to verify the new response body and panic logging behavior.

Changes

Panic Recovery Response Sanitization

Layer / File(s) Summary
Core Behavior
main.go
HTTP 500 response changed from fmt.Sprint(err) (detailed panic message) to fixed "internal server error" message for security and consistency.
Tests
main_test.go
Added wantBody field to test struct and assertions verifying response body matches expected value; enhanced panic-case assertions to check for both "panic!" and the specific panic message in logs.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A panic caught in quiet grace,
No secrets leaked to cyberspace—
Just "error, friend" and logs so neat,
A safer middleware, oh so sweet!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: hide panic details from clients' accurately and concisely describes the main change: preventing panic details from being exposed to clients by returning a generic error message instead.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/hide-panic-details

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coveralls
Copy link
Copy Markdown

Coverage Status

coverage: 79.739%. remained the same — fix/hide-panic-details into main

@raeperd raeperd self-assigned this May 6, 2026
@raeperd raeperd merged commit b58db9e into main May 6, 2026
5 checks passed
@raeperd raeperd deleted the fix/hide-panic-details branch May 6, 2026 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants