diff --git a/README.md b/README.md index 5116a5d..578278c 100644 --- a/README.md +++ b/README.md @@ -233,4 +233,21 @@ fmt.Printf("type: %T; value: %v\n", nString, nString) #### Struct tag directives -Using the struct tag `json:"-"` on a struct field will cause it to be ignored when decoding JSON, even if the JSON input contains a corresponding key/value pair. The `omitzero` and `omitempty` struct tag directives do not have any effect on JSON decoding behavior. \ No newline at end of file +Using the struct tag `json:"-"` on a struct field will cause it to be ignored when decoding JSON, even if the JSON input contains a corresponding key/value pair. The `omitzero` and `omitempty` struct tag directives do not have any effect on JSON decoding behavior. + +#### Error types in decoding json + +| Error Type | Description | +|-------------------------------|-----------------------------------------------------------------------------------------------| +| `json.SyntaxError` | There is a syntax problem with the JSON being decoded. | +| `io.ErrUnexpectedEOF` | There is a syntax problem with the JSON being decoded. | +| `json.UnmarshalTypeError` | A JSON value is not appropriate for the destination Go type. | +| `json.InvalidUnmarshalError` | The decode destination is not valid (usually because it is not a pointer). This is actually a problem with our application code, not the JSON itself. | +| `io.EOF` | The JSON being decoded is empty. | + +Using the `errors.Is()` and `errors.As()` functions you can handle these errors and not expose internal details about your service. A common approach is to build a readJSON message that can be used to traige errors. + +| Function | Purpose | Checks for | Example Use Case | +|-------------|------------------------------------------|----------------|-----------------------------------------| +| `errors.Is` | Compare error values (with wrapping) | Specific value | `errors.Is(err, io.EOF)` | +| `errors.As` | Check and extract a specific error type | Specific type | `errors.As(err, *json.SyntaxError)` | diff --git a/cmd/api/handlers.go b/cmd/api/handlers.go index 170c70f..9554b59 100644 --- a/cmd/api/handlers.go +++ b/cmd/api/handlers.go @@ -1,7 +1,6 @@ package main import ( - "encoding/json" "fmt" "net/http" "strconv" @@ -27,7 +26,7 @@ func (app *application) createMovieHandler(w http.ResponseWriter, r *http.Reques // reads from the request body and decodes to our input struct // must be a non-nil pointer. Otherwise will raise json.InvalidUnmarshalError - err := json.NewDecoder(r.Body).Decode(&input) + err := app.readJSON(w, r, &input) if err != nil { app.errorResponse(w, r, http.StatusBadRequest, err.Error()) return @@ -37,6 +36,7 @@ func (app *application) createMovieHandler(w http.ResponseWriter, r *http.Reques // Dump the contents of the input struct in a HTTP response // +v is the default format value plus field names for structs + w.WriteHeader(http.StatusCreated) fmt.Fprintf(w, "%+v\n", input) } diff --git a/cmd/api/handlers_test.go b/cmd/api/handlers_test.go index dad79e9..250baf6 100644 --- a/cmd/api/handlers_test.go +++ b/cmd/api/handlers_test.go @@ -8,6 +8,7 @@ import ( "log/slog" "net/http" "net/http/httptest" + "strings" "testing" "git.runcible.io/learning/pulley/internal/assert" @@ -47,9 +48,10 @@ func TestHealthRoute(t *testing.T) { func TestCreateMovieHandler(t *testing.T) { respRec := httptest.NewRecorder() - want := "Resource created" - r, err := http.NewRequest(http.MethodPost, "/v1/movies", nil) + requestBody := `{"title": "Moana", "year": 2019, "runtime": 120, "genres": ["family", "Samoan"]}` + + r, err := http.NewRequest(http.MethodPost, "/v1/movies", strings.NewReader(requestBody)) if err != nil { t.Fatal(err) } @@ -59,9 +61,8 @@ func TestCreateMovieHandler(t *testing.T) { resp := respRec.Result() - if resp.StatusCode != http.StatusAccepted { - t.Fatalf("Got status code %q, wanted status code %q", resp.StatusCode, http.StatusOK) - } + assert.Equal(t, resp.StatusCode, http.StatusCreated) + defer resp.Body.Close() body, err := io.ReadAll(resp.Body) @@ -69,7 +70,74 @@ func TestCreateMovieHandler(t *testing.T) { body = bytes.TrimSpace(body) - assert.StringContains(t, string(body), want) + assert.StringContains(t, string(body), "Moana") +} + +// Consider simply testing app.jsonReader +func TestCreateMovieError(t *testing.T) { + + tests := []struct { + name string + input *strings.Reader + wantBody string + wantCode int + }{ + { + name: "Test XML", + input: strings.NewReader(`Alex`), + wantBody: "body contains badly-formed JSON", + wantCode: http.StatusBadRequest, + }, + { + name: "Test Bad JSON", + input: strings.NewReader(`{"title": "Moana", }`), + wantBody: "body contains badly-formed JSON", + wantCode: http.StatusBadRequest, + }, + { + name: "Send a JSON array instead of an object", + input: strings.NewReader(`["not", "good"]`), + wantBody: "body contains incorrect JSON type", + wantCode: http.StatusBadRequest, + }, + { + name: "Send a numeric 'title' value", + input: strings.NewReader(`{"title": 123}`), + wantBody: "body contains incorrect JSON type", + wantCode: http.StatusBadRequest, + }, + { + name: "Send an empty request body", + input: strings.NewReader(""), + wantBody: "body must not be empty", + wantCode: http.StatusBadRequest, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + respRec := httptest.NewRecorder() + r, err := http.NewRequest(http.MethodPost, "/v1/movies", test.input) + if err != nil { + t.Fatal(err) + } + + app := newTestApplication() + + app.routes().ServeHTTP(respRec, r) + + resp := respRec.Result() + + assert.Equal(t, resp.StatusCode, test.wantCode) + + var jsonResp map[string]string + json.NewDecoder(resp.Body).Decode(&jsonResp) + + assert.StringContains(t, jsonResp["error"], test.wantBody) + + }) + } + } func TestGetAllMoviesHandler(t *testing.T) { diff --git a/cmd/api/helpers.go b/cmd/api/helpers.go index ebffe31..759b281 100644 --- a/cmd/api/helpers.go +++ b/cmd/api/helpers.go @@ -1,7 +1,14 @@ +// When Go is encoding a particular type to JSON, +// it looks to see if the type has a MarshalJSON() +// method implemented on it. If it has, then Go will +// call this method to determine how to encode it. package main import ( "encoding/json" + "errors" + "fmt" + "io" "net/http" ) @@ -42,3 +49,57 @@ func (app *application) writeJSON(w http.ResponseWriter, status int, data envelo return nil } + +func (app *application) readJSON(w http.ResponseWriter, r *http.Request, dst any) error { + // Decode the request body into the target destination. + err := json.NewDecoder(r.Body).Decode(dst) + if err != nil { + // If there is an error during decoding, start the triage... + var syntaxError *json.SyntaxError + var unmarshalTypeError *json.UnmarshalTypeError + var invalidUnmarshalError *json.InvalidUnmarshalError + + switch { + // Use the errors.As() function to check whether the error has the type + // *json.SyntaxError. If it does, then return a plain-english error message + // which includes the location of the problem. + case errors.As(err, &syntaxError): + return fmt.Errorf("body contains badly-formed JSON (at character %d)", syntaxError.Offset) + + // In some circumstances Decode() may also return an io.ErrUnexpectedEOF error + // for syntax errors in the JSON. So we check for this using errors.Is() and + // return a generic error message. There is an open issue regarding this at + // https://github.com/golang/go/issues/25956. + case errors.Is(err, io.ErrUnexpectedEOF): + return errors.New("body contains badly-formed JSON") + + // Likewise, catch any *json.UnmarshalTypeError errors. These occur when the + // JSON value is the wrong type for the target destination. If the error relates + // to a specific field, then we include that in our error message to make it + // easier for the client to debug. + case errors.As(err, &unmarshalTypeError): + if unmarshalTypeError.Field != "" { + return fmt.Errorf("body contains incorrect JSON type for field %q", unmarshalTypeError.Field) + } + return fmt.Errorf("body contains incorrect JSON type (at character %d)", unmarshalTypeError.Offset) + + // An io.EOF error will be returned by Decode() if the request body is empty. We + // check for this with errors.Is() and return a plain-english error message + // instead. + case errors.Is(err, io.EOF): + return errors.New("body must not be empty") + + // A json.InvalidUnmarshalError error will be returned if we pass something + // that is not a non-nil pointer to Decode(). We catch this and panic, + // rather than returning an error to our handler. + case errors.As(err, &invalidUnmarshalError): + panic(err) + + // For anything else, return the error message as-is. + default: + return err + } + } + + return nil +}