-
Notifications
You must be signed in to change notification settings - Fork 54
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
refactor: Improve the HTTP API implementation #382
Changes from all commits
6a6da75
f7ad42f
60edcc3
fa67697
722ae12
4e6600b
38ca252
f2ffd8b
9bb07d5
64aad49
871eebf
6dcbf1d
5209812
45a4533
695a0f0
fa44712
55c9bcf
f021c46
56862b5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
// Copyright 2022 Democratized Data Foundation | ||
// | ||
// Use of this software is governed by the Business Source License | ||
// included in the file licenses/BSL.txt. | ||
// | ||
// As of the Change Date specified in that file, in accordance with | ||
// the Business Source License, use of this software will be governed | ||
// by the Apache License, Version 2.0, included in the file | ||
// licenses/APL.txt. | ||
|
||
package http | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
"net/http" | ||
"os" | ||
"strings" | ||
) | ||
|
||
var env = os.Getenv("DEFRA_ENV") | ||
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. suggestion: I think this is very tucked away for a public concept, would suggest making the variable public ( 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.
Makes sense to make it public if the config file or cli flags can set the variable. Although if that is the case maybe it should be moved to the WIP config package. I'll leave it as is for now and create a pull request to make the change once the config package is merged. |
||
|
||
type errorResponse struct { | ||
Status int `json:"status"` | ||
Message string `json:"message"` | ||
Stack string `json:"stack,omitempty"` | ||
} | ||
|
||
func handleErr(ctx context.Context, rw http.ResponseWriter, err error, status int) { | ||
if status == http.StatusInternalServerError { | ||
log.ErrorE(ctx, http.StatusText(status), err) | ||
} | ||
|
||
sendJSON( | ||
ctx, | ||
rw, | ||
errorResponse{ | ||
Status: status, | ||
Message: http.StatusText(status), | ||
Stack: formatError(err), | ||
}, | ||
status, | ||
) | ||
} | ||
|
||
func formatError(err error) string { | ||
if strings.ToLower(env) == "dev" || strings.ToLower(env) == "development" { | ||
return fmt.Sprintf("[DEV] %+v\n", err) | ||
} | ||
return "" | ||
} |
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.
😄