Conversation
- Add responsive error page template with embedded CSS styling - Implement error wrapper functions for different route types - Add conditional 'Go Back' button for user-facing routes - Maintain fallback support for plain text errors - Add comprehensive error page documentation 🤖 Generated with GitHub Copilot (via Zed) Signed-off-by: Eric Searcy <eric@linuxfoundation.org>
Apparently due to an error in the securecookie library, the error text became more descriptive, causing warnings about cookie length to fall through to the generic "session error". This changes to error string matching to a prefix match, while also providing the user a more helpful message. Signed-off-by: Eric Searcy <eric@linuxfoundation.org>
Signed-off-by: Eric Searcy <eric@linuxfoundation.org>
There was a problem hiding this comment.
Pull Request Overview
This PR implements a user-friendly HTML error page system to replace plain text error messages across the application. The changes introduce a responsive error template with embedded CSS styling, differentiate error handling between user-facing routes and CAS protocol routes, and maintain backward compatibility through fallback mechanisms.
Key changes:
- Added responsive HTML error template with conditional "Go Back" button functionality
- Implemented route-specific error rendering functions (
renderUserErrorPage,renderCallbackErrorPage) - Replaced direct
http.Errorcalls with template-based error pages for user-facing routes - Fixed cookie length warning check to use prefix matching instead of exact comparison
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| templates/error.html | New responsive HTML error page template with embedded CSS and conditional back button |
| responses.go | Added error template initialization, error rendering functions, and moved outputFailure from cas.go |
| cas.go | Replaced http.Error calls with new error rendering functions and improved cookie error handling |
| ERROR_PAGES.md | Comprehensive documentation of error page system architecture and usage |
| Dockerfile | Added templates directory to Docker image |
| docker-compose.yaml | Removed deprecated version field |
| .mega-linter.yml | Disabled HTML linters |
| .cspell.json | Added .mega-linter.yml to ignore paths |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| // Fall back to plain text error if no template is available. | ||
| w.Header().Set("Content-Type", "text/plain; charset=utf-8") | ||
| w.WriteHeader(statusCode) | ||
| w.Write([]byte(message)) |
There was a problem hiding this comment.
The return value of w.Write is not checked. While this is a fallback error case, consider handling the error: if _, err := w.Write([]byte(message)); err != nil { appLogger(ctx).Error(\"error writing fallback error\", \"error\", err) }
| w.Write([]byte(message)) | |
| if _, err := w.Write([]byte(message)); err != nil { | |
| appLogger(ctx).Error("error writing fallback error", "error", err) | |
| } |
| @@ -104,17 +104,17 @@ func casLogin(w http.ResponseWriter, r *http.Request) { | |||
| session, _ := store.Get(r, "cas-shim") | |||
| session.Values[state] = service | |||
| err = session.Save(r, w) | |||
There was a problem hiding this comment.
Using strings.HasPrefix on error messages is fragile as it depends on error text formatting. Consider checking for a specific error type if available from the securecookie library, or document why prefix matching is necessary instead of exact matching.
| err = session.Save(r, w) | |
| err = session.Save(r, w) | |
| // NOTE: The securecookie library does not export a specific error type or variable for this condition. | |
| // Therefore, we must match the error message prefix. If a specific error type becomes available in the future, | |
| // this should be updated to use errors.Is or a type assertion. |
Signed-off-by: Eric Searcy <eric@linuxfoundation.org>
Signed-off-by: Eric Searcy <eric@linuxfoundation.org>
Implement user-friendly HTML error page system
🤖 Assisted with GitHub Copilot (via Zed)