From 7c79b19228ba2bc07b4704a5f849055a3621ae64 Mon Sep 17 00:00:00 2001 From: manuteleco Date: Thu, 14 Mar 2024 00:48:05 +0100 Subject: [PATCH] Fill the `errors` attribute for HTTP errors (#1528) 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 --- fakestorage/json_response.go | 14 +++++++++++++- fakestorage/object_test.go | 2 +- fakestorage/upload_test.go | 4 ++-- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/fakestorage/json_response.go b/fakestorage/json_response.go index f16a7c5c10..99e8ce7d4c 100644 --- a/fakestorage/json_response.go +++ b/fakestorage/json_response.go @@ -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 } @@ -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 diff --git a/fakestorage/object_test.go b/fakestorage/object_test.go index b1581583ed..1c11a3190f 100644 --- a/fakestorage/object_test.go +++ b/fakestorage/object_test.go @@ -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 { diff --git a/fakestorage/upload_test.go b/fakestorage/upload_test.go index 79908fa42d..582c913e35 100644 --- a/fakestorage/upload_test.go +++ b/fakestorage/upload_test.go @@ -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) } @@ -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) }