Skip to content

Commit

Permalink
Fill the errors attribute for HTTP errors (#1528)
Browse files Browse the repository at this point in the history
Some GCS client libraries (like [this one][1]), are quite strict when it
comes to the set of attributes they expect to find in the error
response. If any of those attributes is missing, they fail to
deserialize the error.

In `fake-gcs-server`, the `httpError.Errors` attribute was always
initialized to `nil`, causing the `error.errors` JSON response attribute
to be omitted and leading to failures in some client libraries.

In this commit we address that issue by always providing content on the
`errors` attribute (`httpError.Errors` field) whenever we return an
error response. The implementation is kept simple without aiming to
return exactly the same values that are [officially documented][2]. The
main goal is to have a sensible response that is _structurally_ correct
for the purposes of deserialization client-side.

[1]: https://crates.io/crates/google-cloud-storage
[2]: https://cloud.google.com/storage/docs/json_api/v1/status-codes
  • Loading branch information
manuteleco authored Mar 13, 2024
1 parent 447880e commit 7c79b19
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 4 deletions.
14 changes: 13 additions & 1 deletion fakestorage/json_response.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func jsonToHTTPHandler(h jsonHandler) http.HandlerFunc {
status := resp.getStatus()
var data any
if status > 399 {
data = newErrorResponse(status, resp.getErrorMessage(status), nil)
data = newErrorResponse(status, resp.getErrorMessage(status), resp.getErrorList(status))
} else {
data = resp.data
}
Expand All @@ -59,6 +59,18 @@ func (r *jsonResponse) getErrorMessage(status int) string {
return http.StatusText(status)
}

func (r *jsonResponse) getErrorList(status int) []apiError {
if status == http.StatusOK {
return nil
} else {
return []apiError{{
Domain: "global",
Reason: http.StatusText(status),
Message: r.getErrorMessage(status),
}}
}
}

func errToJsonResponse(err error) jsonResponse {
status := 0
var pathError *os.PathError
Expand Down
2 changes: 1 addition & 1 deletion fakestorage/object_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2028,7 +2028,7 @@ func TestServiceClientComposeObject(t *testing.T) {
"files/destination.txt",
[]string{"01", "02", "03", "04", "05", "06", "07", "08", "09", "10", "11", "12", "13", "14", "15", "16", "17", "18", "19", "20", "21", "22", "23", "24", "25", "26", "27", "28", "29", "30", "31", "32", "33"},
"",
"googleapi: Error 400: The number of source components provided (33) exceeds the maximum (32)",
"googleapi: Error 400: The number of source components provided (33) exceeds the maximum (32), Bad Request",
},
}
for _, test := range tests {
Expand Down
4 changes: 2 additions & 2 deletions fakestorage/upload_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ func TestServerClientObjectWriterWithDoesNotExistPrecondition(t *testing.T) {
if err == nil {
t.Fatal("expected overwriting existing object to fail, but received no error")
}
if err.Error() != "googleapi: Error 412: Precondition failed" {
if err.Error() != "googleapi: Error 412: Precondition failed, Precondition Failed" {
t.Errorf("expected HTTP 412 precondition failed error, but got %v", err)
}

Expand Down Expand Up @@ -346,7 +346,7 @@ func TestServerClientObjectOperationsWithIfGenerationNotMatchPrecondition(t *tes
if err == nil {
t.Fatal("expected overwriting existing object to fail, but received no error")
}
if err.Error() != "googleapi: Error 412: Precondition failed" {
if err.Error() != "googleapi: Error 412: Precondition failed, Precondition Failed" {
t.Errorf("expected HTTP 412 precondition failed error, but got %v", err)
}

Expand Down

0 comments on commit 7c79b19

Please sign in to comment.