-
Notifications
You must be signed in to change notification settings - Fork 3
chore: centralized error formatting #866
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,63 @@ | ||
| package revproxy | ||
|
|
||
| import ( | ||
| "errors" | ||
| "fmt" | ||
| "net/http" | ||
| "strings" | ||
|
|
||
| "github.com/labstack/echo/v4" | ||
| ) | ||
|
|
||
| // The same error content as the data services API | ||
| const gwBaseErrorCode int = 6000 | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why 6000? (Do we have a rule for these numbers?) |
||
|
|
||
| type errorContent struct { | ||
| Code int `json:"code"` | ||
| Message string `json:"message"` | ||
| Detail string `json:"detail,omitempty"` | ||
| TraceId string `json:"trace_id,omitempty"` | ||
| } | ||
|
|
||
| type errorResponse struct { | ||
| Error errorContent `json:"error"` | ||
| } | ||
|
|
||
| // Adapted from https://echo.labstack.com/docs/error-handling | ||
| func ErrorHandler(err error, c echo.Context) { | ||
| if c.Response() != nil && c.Response().Committed { | ||
| return // response has been already sent to the client by handler or some middleware | ||
| } | ||
|
|
||
| accept := c.Request().Header.Get("Accept") | ||
| isHTML := strings.Contains(accept, echo.MIMETextHTML) | ||
|
Comment on lines
+32
to
+33
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remark: weak content negotiation. But there is no standard library method to at least parse the accept header (not even try to perform content negotiation). |
||
|
|
||
| // If the accept header is html then we fall back to the default handler (for now). | ||
| // If the acceptt header is not html or is blank we return json | ||
| if isHTML { | ||
| c.Echo().DefaultHTTPErrorHandler(err, c) | ||
| return | ||
| } | ||
|
|
||
| code := http.StatusInternalServerError | ||
| message := err.Error() | ||
| var he *echo.HTTPError | ||
| if errors.As(err, &he) { // find error in an error chain that implements HTTPError | ||
| if tmp := he.Code; tmp != 0 { | ||
| code = tmp | ||
| } | ||
| if msg := fmt.Sprintf("%v", he.Message); len(msg) > 0 { | ||
| message = msg | ||
| } | ||
| } | ||
|
|
||
| var cErr error | ||
| if c.Request().Method == http.MethodHead { | ||
| cErr = c.NoContent(code) | ||
| } else { | ||
| cErr = c.JSON(code, errorResponse{Error: errorContent{Code: gwBaseErrorCode + code, Message: message}}) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like we are not using |
||
| } | ||
| if cErr != nil { | ||
| c.Logger().Error("failed to send error page to client", "error", errors.Join(err, cErr)) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Use |
||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: maybe this file should be in
internal/gwerrors/?