feat: improve error handling with custom error template #47
Conversation
- Add custom error template for better user experience - Replace c.Error() calls with proper error rendering - Add error handling for session.Save() operations - Remove default session options from main.go
There was a problem hiding this comment.
Pull Request Overview
This PR enhances error handling in the authentication system by introducing a custom error template and implementing proper error handling practices. The changes improve user experience by replacing generic HTTP error responses with user-friendly error pages and ensure session operations are properly error-checked.
- Added a custom HTML error template with modern styling for better user experience
- Replaced generic
c.Error()calls with proper error rendering using a newrenderError()method - Added comprehensive error handling for session save operations to prevent silent failures
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/mcp-proxy/main.go | Removed default session configuration options for cleaner setup |
| pkg/auth/templates/error.html | Added new custom error template with modern styling and error message display |
| pkg/auth/auth.go | Implemented renderError method, added error template parsing, and enhanced session save error handling |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| c.Header("Content-Type", "text/html; charset=utf-8") | ||
| c.Status(http.StatusInternalServerError) | ||
| if templateErr := a.errorTemplate.Execute(c.Writer, data); templateErr != nil { | ||
| c.AbortWithError(http.StatusInternalServerError, templateErr) |
There was a problem hiding this comment.
Using c.AbortWithError() after already setting the status and writing to the response can cause issues. Since the template execution failed, the response may be partially written. Consider using c.String(http.StatusInternalServerError, "Internal Server Error") instead to ensure a clean error response.
| c.AbortWithError(http.StatusInternalServerError, templateErr) | |
| c.String(http.StatusInternalServerError, "Internal Server Error") |
| c.AbortWithError(http.StatusInternalServerError, templateErr) | ||
| return | ||
| } | ||
| c.Abort() |
There was a problem hiding this comment.
The c.Abort() call is unnecessary here since the response has already been completed. The function should return after successfully executing the template. This call could interfere with the response that was just sent.
| c.Abort() |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Summary
This PR improves error handling throughout the authentication system by implementing a custom error template and better error handling practices.
Type of Change
Changes Made
pkg/auth/templates/error.html) for better user experiencec.Error()calls with proper error rendering usingrenderError()methodsession.Save()operations to prevent silent failuresBenefits