Skip to content

chore: Linter rule for properly formatted api errors #2123

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Jun 7, 2022
Merged

chore: Linter rule for properly formatted api errors #2123

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6970d2b
chore: Linter rule for properly formatted api errors
Emyrk Jun 7, 2022
0ca30a0
Fix all linting errors
Emyrk Jun 7, 2022
0e66d8a
Update comment
Emyrk Jun 7, 2022
5134987
Fix error check in test
Emyrk Jun 7, 2022
b813011
fixup! Fix error check in test
Emyrk Jun 7, 2022
173cf26
Remove comment
Emyrk Jun 7, 2022
1b2ef83
Add omitempty to 'Detail' field
Emyrk Jun 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions coderd/coderd.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ func New(options *Options) *API {
)
r.Get("/", func(w http.ResponseWriter, r *http.Request) {
httpapi.Write(w, http.StatusOK, httpapi.Response{
//nolint:gocritic
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good linter 🕵️🕵️🕵️

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The only message I needed to make an exception for 😄

Message: "👋",
})
})
Expand Down
2 changes: 1 addition & 1 deletion coderd/csp.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ func (api *API) logReportCSPViolations(rw http.ResponseWriter, r *http.Request)
if err != nil {
api.Logger.Warn(ctx, "csp violation", slog.Error(err))
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
Message: "Failed to read body, invalid json",
Message: "Failed to read body, invalid json.",
Detail: err.Error(),
})
return
Expand Down
10 changes: 5 additions & 5 deletions coderd/files.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func (api *API) postFile(rw http.ResponseWriter, r *http.Request) {
case "application/x-tar":
default:
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
Message: fmt.Sprintf("Unsupported content type header %q", contentType),
Message: fmt.Sprintf("Unsupported content type header %q.", contentType),
})
return
}
Expand All @@ -41,7 +41,7 @@ func (api *API) postFile(rw http.ResponseWriter, r *http.Request) {
data, err := io.ReadAll(r.Body)
if err != nil {
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
Message: "Failed to read file from request",
Message: "Failed to read file from request.",
Detail: err.Error(),
})
return
Expand All @@ -65,7 +65,7 @@ func (api *API) postFile(rw http.ResponseWriter, r *http.Request) {
})
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: "Internal error saving file",
Message: "Internal error saving file.",
Detail: err.Error(),
})
return
Expand All @@ -80,7 +80,7 @@ func (api *API) fileByHash(rw http.ResponseWriter, r *http.Request) {
hash := chi.URLParam(r, "hash")
if hash == "" {
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
Message: "File hash must be provided in url",
Message: "File hash must be provided in url.",
})
return
}
Expand All @@ -91,7 +91,7 @@ func (api *API) fileByHash(rw http.ResponseWriter, r *http.Request) {
}
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: "Internal error fetching file",
Message: "Internal error fetching file.",
Detail: err.Error(),
})
return
Expand Down
16 changes: 8 additions & 8 deletions coderd/gitsshkey.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ func (api *API) regenerateGitSSHKey(rw http.ResponseWriter, r *http.Request) {
privateKey, publicKey, err := gitsshkey.Generate(api.SSHKeygenAlgorithm)
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: "Internal error generating a new SSH keypair",
Message: "Internal error generating a new SSH keypair.",
Detail: err.Error(),
})
return
Expand All @@ -35,7 +35,7 @@ func (api *API) regenerateGitSSHKey(rw http.ResponseWriter, r *http.Request) {
})
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: "Internal error updating user's git SSH key",
Message: "Internal error updating user's git SSH key.",
Detail: err.Error(),
})
return
Expand All @@ -44,7 +44,7 @@ func (api *API) regenerateGitSSHKey(rw http.ResponseWriter, r *http.Request) {
newKey, err := api.Database.GetGitSSHKey(r.Context(), user.ID)
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: "Internal error fetching user's git SSH key",
Message: "Internal error fetching user's git SSH key.",
Detail: err.Error(),
})
return
Expand All @@ -69,7 +69,7 @@ func (api *API) gitSSHKey(rw http.ResponseWriter, r *http.Request) {
gitSSHKey, err := api.Database.GetGitSSHKey(r.Context(), user.ID)
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: "Internal error fetching user's SSH key",
Message: "Internal error fetching user's SSH key.",
Detail: err.Error(),
})
return
Expand All @@ -89,7 +89,7 @@ func (api *API) agentGitSSHKey(rw http.ResponseWriter, r *http.Request) {
resource, err := api.Database.GetWorkspaceResourceByID(r.Context(), agent.ResourceID)
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: "Internal error fetching workspace resource",
Message: "Internal error fetching workspace resource.",
Detail: err.Error(),
})
return
Expand All @@ -98,7 +98,7 @@ func (api *API) agentGitSSHKey(rw http.ResponseWriter, r *http.Request) {
job, err := api.Database.GetWorkspaceBuildByJobID(r.Context(), resource.JobID)
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: "Internal error fetching workspace build",
Message: "Internal error fetching workspace build.",
Detail: err.Error(),
})
return
Expand All @@ -107,7 +107,7 @@ func (api *API) agentGitSSHKey(rw http.ResponseWriter, r *http.Request) {
workspace, err := api.Database.GetWorkspaceByID(r.Context(), job.WorkspaceID)
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: "Internal error fetching workspace",
Message: "Internal error fetching workspace.",
Detail: err.Error(),
})
return
Expand All @@ -116,7 +116,7 @@ func (api *API) agentGitSSHKey(rw http.ResponseWriter, r *http.Request) {
gitSSHKey, err := api.Database.GetGitSSHKey(r.Context(), workspace.OwnerID)
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: "Internal error fetching git SSH key",
Message: "Internal error fetching git SSH key.",
Detail: err.Error(),
})
return
Expand Down
10 changes: 5 additions & 5 deletions coderd/httpapi/httpapi.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ type Response struct {
// err.Error() text.
// - "database: too many open connections"
// - "stat: too many open files"
Detail string `json:"detail"`
Detail string `json:"detail,omitempty"`
// Validations are form field-specific friendly error messages. They will be
// shown on a form field in the UI. These can also be used to add additional
// context if there is a set of errors in the primary 'Message'.
Expand All @@ -78,7 +78,7 @@ type Error struct {

func Forbidden(rw http.ResponseWriter) {
Write(rw, http.StatusForbidden, Response{
Message: "Forbidden",
Message: "Forbidden.",
})
}

Expand Down Expand Up @@ -107,7 +107,7 @@ func Read(rw http.ResponseWriter, r *http.Request, value interface{}) bool {
err := json.NewDecoder(r.Body).Decode(value)
if err != nil {
Write(rw, http.StatusBadRequest, Response{
Message: "Request body must be valid JSON",
Message: "Request body must be valid JSON.",
Detail: err.Error(),
})
return false
Expand All @@ -123,14 +123,14 @@ func Read(rw http.ResponseWriter, r *http.Request, value interface{}) bool {
})
}
Write(rw, http.StatusBadRequest, Response{
Message: "Validation failed",
Message: "Validation failed.",
Validations: apiErrors,
})
return false
}
if err != nil {
Write(rw, http.StatusInternalServerError, Response{
Message: "Internal error validating request body payload",
Message: "Internal error validating request body payload.",
Detail: err.Error(),
})
return false
Expand Down
2 changes: 1 addition & 1 deletion coderd/httpapi/httpapi_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func TestWrite(t *testing.T) {
t.Parallel()
rw := httptest.NewRecorder()
httpapi.Write(rw, http.StatusOK, httpapi.Response{
Message: "wow",
Message: "Wow.",
})
var m map[string]interface{}
err := json.NewDecoder(rw.Body).Decode(&m)
Expand Down
24 changes: 12 additions & 12 deletions coderd/httpmw/apikey.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,15 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs) func(http.Handler) h
}
if cookieValue == "" {
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
Message: fmt.Sprintf("Cookie %q or query parameter must be provided", SessionTokenKey),
Message: fmt.Sprintf("Cookie %q or query parameter must be provided.", SessionTokenKey),
})
return
}
parts := strings.Split(cookieValue, "-")
// APIKeys are formatted: ID-SECRET
if len(parts) != 2 {
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
Message: fmt.Sprintf("Invalid %q cookie API key format", SessionTokenKey),
Message: fmt.Sprintf("Invalid %q cookie API key format.", SessionTokenKey),
})
return
}
Expand All @@ -82,26 +82,26 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs) func(http.Handler) h
// Ensuring key lengths are valid.
if len(keyID) != 10 {
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
Message: fmt.Sprintf("Invalid %q cookie API key id", SessionTokenKey),
Message: fmt.Sprintf("Invalid %q cookie API key id.", SessionTokenKey),
})
return
}
if len(keySecret) != 22 {
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
Message: fmt.Sprintf("Invalid %q cookie API key secret", SessionTokenKey),
Message: fmt.Sprintf("Invalid %q cookie API key secret.", SessionTokenKey),
})
return
}
key, err := db.GetAPIKeyByID(r.Context(), keyID)
if err != nil {
if errors.Is(err, sql.ErrNoRows) {
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
Message: "API key is invalid",
Message: "API key is invalid.",
})
return
}
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: "Internal error fetching API key by id",
Message: "Internal error fetching API key by id.",
Detail: err.Error(),
})
return
Expand All @@ -111,7 +111,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs) func(http.Handler) h
// Checking to see if the secret is valid.
if subtle.ConstantTimeCompare(key.HashedSecret, hashed[:]) != 1 {
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
Message: "API key secret is invalid",
Message: "API key secret is invalid.",
})
return
}
Expand All @@ -128,7 +128,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs) func(http.Handler) h
oauthConfig = oauth.Github
default:
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: fmt.Sprintf("Unexpected authentication type %q", key.LoginType),
Message: fmt.Sprintf("Unexpected authentication type %q.", key.LoginType),
})
return
}
Expand All @@ -140,7 +140,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs) func(http.Handler) h
}).Token()
if err != nil {
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
Message: "Could not refresh expired Oauth token",
Message: "Could not refresh expired Oauth token.",
Detail: err.Error(),
})
return
Expand All @@ -156,7 +156,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs) func(http.Handler) h
// Checking if the key is expired.
if key.ExpiresAt.Before(now) {
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
Message: fmt.Sprintf("API key expired at %q", key.ExpiresAt.String()),
Message: fmt.Sprintf("API key expired at %q.", key.ExpiresAt.String()),
})
return
}
Expand Down Expand Up @@ -184,7 +184,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs) func(http.Handler) h
})
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: fmt.Sprintf("API key couldn't update: %s", err.Error()),
Message: fmt.Sprintf("API key couldn't update: %s.", err.Error()),
})
return
}
Expand All @@ -196,7 +196,7 @@ func ExtractAPIKey(db database.Store, oauth *OAuth2Configs) func(http.Handler) h
roles, err := db.GetAuthorizationUserRoles(r.Context(), key.UserID)
if err != nil {
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
Message: "Internal error fetching user's roles",
Message: "Internal error fetching user's roles.",
Detail: err.Error(),
})
return
Expand Down
6 changes: 3 additions & 3 deletions coderd/httpmw/apikey_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func TestAPIKey(t *testing.T) {
successHandler := http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
// Only called if the API key passes through the handler.
httpapi.Write(rw, http.StatusOK, httpapi.Response{
Message: "it worked!",
Message: "It worked!",
})
})

Expand Down Expand Up @@ -203,7 +203,7 @@ func TestAPIKey(t *testing.T) {
// Checks that it exists on the context!
_ = httpmw.APIKey(r)
httpapi.Write(rw, http.StatusOK, httpapi.Response{
Message: "it worked!",
Message: "It worked!",
})
})).ServeHTTP(rw, r)
res := rw.Result()
Expand Down Expand Up @@ -241,7 +241,7 @@ func TestAPIKey(t *testing.T) {
// Checks that it exists on the context!
_ = httpmw.APIKey(r)
httpapi.Write(rw, http.StatusOK, httpapi.Response{
Message: "it worked!",
Message: "It worked!",
})
})).ServeHTTP(rw, r)
res := rw.Result()
Expand Down
4 changes: 2 additions & 2 deletions coderd/httpmw/httpmw.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ func parseUUID(rw http.ResponseWriter, r *http.Request, param string) (uuid.UUID
rawID := chi.URLParam(r, param)
if rawID == "" {
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
Message: "Missing UUID in URL",
Message: "Missing UUID in URL.",
// Url params mean nothing to a user
Detail: fmt.Sprintf("%q URL param missing", param),
})
Expand All @@ -25,7 +25,7 @@ func parseUUID(rw http.ResponseWriter, r *http.Request, param string) (uuid.UUID
parsed, err := uuid.Parse(rawID)
if err != nil {
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
Message: fmt.Sprintf("Invalid UUID %q", param),
Message: fmt.Sprintf("Invalid UUID %q.", param),
Detail: err.Error(),
})
return uuid.UUID{}, false
Expand Down
10 changes: 5 additions & 5 deletions coderd/httpmw/oauth2.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ func ExtractOAuth2(config OAuth2Config) func(http.Handler) http.Handler {
state, err := cryptorand.String(32)
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: "Internal error generating state string",
Message: "Internal error generating state string.",
Detail: err.Error(),
})
return
Expand Down Expand Up @@ -92,21 +92,21 @@ func ExtractOAuth2(config OAuth2Config) func(http.Handler) http.Handler {

if state == "" {
httpapi.Write(rw, http.StatusBadRequest, httpapi.Response{
Message: "State must be provided",
Message: "State must be provided.",
})
return
}

stateCookie, err := r.Cookie(oauth2StateCookieName)
if err != nil {
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
Message: fmt.Sprintf("Cookie %q must be provided", oauth2StateCookieName),
Message: fmt.Sprintf("Cookie %q must be provided.", oauth2StateCookieName),
})
return
}
if stateCookie.Value != state {
httpapi.Write(rw, http.StatusUnauthorized, httpapi.Response{
Message: "State mismatched",
Message: "State mismatched.",
})
return
}
Expand All @@ -120,7 +120,7 @@ func ExtractOAuth2(config OAuth2Config) func(http.Handler) http.Handler {
oauthToken, err := config.Exchange(r.Context(), code)
if err != nil {
httpapi.Write(rw, http.StatusInternalServerError, httpapi.Response{
Message: "Internal error exchanging Oauth code",
Message: "Internal error exchanging Oauth code.",
Detail: err.Error(),
})
return
Expand Down
Loading