REST API Mistakes That Keep Showing Up in Go Code Reviews

Code review catches the interesting bugs and lets the boring ones through. The boring ones are the ones that ship: a handler that returns the wrong status in one error path, a list that is null when empty, a missing return. They share a feature. Each produces a response that looks reasonable when you test the happy path, and the failure shows up weeks later in a client's logs.

This is a collection of the mistakes that come up most often in Go REST handlers, each reproduced so you can see exactly what the client receives, with the fix beside it. The reproductions use one program that ran on Go 1.25.6. The longer treatments are in designing a REST API, response structure and HTTP error handling.

package main

import (
	"encoding/json"
	"fmt"
	"io"
	"log"
	"net/http"
	"net/http/httptest"
	"strings"
)

func show(label string, h http.HandlerFunc) {
	srv := httptest.NewServer(h)
	defer srv.Close()
	resp, err := http.Get(srv.URL)
	if err != nil {
		fmt.Printf("%-44s error: %v\n", label, err)
		return
	}
	b, _ := io.ReadAll(resp.Body)
	resp.Body.Close()
	fmt.Printf("%-44s %d %-16s %q\n", label, resp.StatusCode, resp.Header.Get("Content-Type"), strings.TrimSpace(string(b)))
}

type Page struct {
	Items []string `json:"items"`
}

func main() {
	log.SetFlags(0)
	log.SetOutput(io.Discard) // the server logs "superfluous WriteHeader"; keep the demo output clean

	// 1. Status written after the body: the 500 never happens.
	show("1a. WriteHeader after Write", func(w http.ResponseWriter, r *http.Request) {
		fmt.Fprint(w, "partial result")
		w.WriteHeader(http.StatusInternalServerError)
	})

	// 2. Forgetting to return after replying with an error.
	show("2a. missing return after an error", func(w http.ResponseWriter, r *http.Request) {
		if r.URL.Query().Get("id") == "" {
			http.Error(w, "id required", http.StatusBadRequest)
		}
		fmt.Fprint(w, "secret data for everyone")
	})

	// 3. Ignoring the error from Encode. The value cannot be encoded, nothing is written, and the client gets an empty 200.
	show("3a. Encode error ignored", func(w http.ResponseWriter, r *http.Request) {
		w.Header().Set("Content-Type", "application/json")
		json.NewEncoder(w).Encode(map[string]any{"bad": make(chan int)})
	})
	show("3b. Encode error checked", func(w http.ResponseWriter, r *http.Request) {
		b, err := json.Marshal(map[string]any{"bad": make(chan int)})
		if err != nil {
			w.Header().Set("Content-Type", "application/json")
			w.WriteHeader(http.StatusInternalServerError)
			io.WriteString(w, `{"error":"encode failed"}`)
			return
		}
		w.Write(b)
	})

	// 4. An empty list that serialises as null.
	show("4a. nil slice", func(w http.ResponseWriter, r *http.Request) {
		var p Page
		json.NewEncoder(w).Encode(p)
	})
	show("4b. initialised slice", func(w http.ResponseWriter, r *http.Request) {
		json.NewEncoder(w).Encode(Page{Items: []string{}})
	})

	// 5. http.Error on a JSON API.
	show("5a. http.Error with JSON text", func(w http.ResponseWriter, r *http.Request) {
		http.Error(w, `{"error":"nope"}`, http.StatusBadRequest)
	})
	show("5b. explicit content type", func(w http.ResponseWriter, r *http.Request) {
		w.Header().Set("Content-Type", "application/json")
		w.WriteHeader(http.StatusBadRequest)
		io.WriteString(w, `{"error":"nope"}`)
	})

	// 6. Headers set after the first write are lost.
	show("6a. header set after Write", func(w http.ResponseWriter, r *http.Request) {
		fmt.Fprint(w, `{"ok":true}`)
		w.Header().Set("Content-Type", "application/json")
	})
}

1a. WriteHeader after Write                  200 text/plain; charset=utf-8 "partial result"
2a. missing return after an error            400 text/plain; charset=utf-8 "id required\nsecret data for everyone"
3a. Encode error ignored                     200 application/json ""
3b. Encode error checked                     500 application/json "{\"error\":\"encode failed\"}"
4a. nil slice                                200 text/plain; charset=utf-8 "{\"items\":null}"
4b. initialised slice                        200 text/plain; charset=utf-8 "{\"items\":[]}"
5a. http.Error with JSON text                400 text/plain; charset=utf-8 "{\"error\":\"nope\"}"
5b. explicit content type                    400 application/json "{\"error\":\"nope\"}"
6a. header set after Write                   200 text/plain; charset=utf-8 "{\"ok\":true}"

Mistakes in the response

1. Writing the status after the body. The first row of the output: the handler wrote partial result and then called WriteHeader(500). The client got a 200. The first call to Write sends the headers with an implicit 200, and later calls to WriteHeader are ignored (the server logs "superfluous response.WriteHeader call"). Set the status before writing anything, and when something can fail after you have started writing, produce the body in memory first so the decision is made before the first byte goes out.

2. Setting headers after the first write. The last row: Content-Type was set after Fprint, and the response went out as text/plain. Headers are sent with the first write, so everything you want in them must be set before it. When a handler grows and someone moves a line, this one breaks silently.

3. Using http.Error for JSON. http.Error is a convenience for plain text. It sets Content-Type: text/plain; charset=utf-8 and X-Content-Type-Options: nosniff, and writes the message followed by a newline. Pass it a JSON string and you get a JSON body labelled as text, which clients that check the content type will not parse. Rows 5a and 5b show the difference. Write a small helper that sets application/json (or application/problem+json), the status and the body, and use it everywhere (problem details).

4. Ignoring the error from Encode. Row 3a: encoding a map containing a channel failed, nothing was written, and the handler ignored the returned error. The client received 200 with Content-Type: application/json and an empty body, and a JSON parser reports "unexpected end of input" at the other end. Row 3b shows the repair: marshal first, check the error, and answer 500 with a real body. Encoder buffers a single value before writing, so an unsupported type is detected before any bytes go out. Errors during the write itself (the client disconnected) cannot be repaired and are worth logging at debug level.

5. Empty lists that are null. A nil slice marshals as null, an empty one as []. Row 4a is {"items":null}. A JavaScript client that does response.items.map(...) throws. Initialise slices that go to JSON (Items: []string{}), or normalise in a response constructor. The same applies to maps. See null versus omitted.

6. Returning 200 with an error in the body. {"success": false, "error": "not found"} with status 200 defeats every proxy, monitor and client library that uses status codes. Use the right status (status code reference) and put detail in the body.

Mistakes in control flow

7. Missing return after an error reply. Row 2a is the dangerous one. The handler detected a missing id, called http.Error with a 400 and then continued to write the data. The status was 400, and the body was id required followed by secret data for everyone. In a real handler the "data" is a database result, and the bug turns a validation failure into a data leak (and also an unnecessary query). Write error replies as http.Error(...); return, or use a helper that makes returning the natural next line. Linters that detect code after a terminating call help; review habits help more.

8. Panics as flow control. A panic in a handler is recovered by the server and logged, and the client sees a dropped connection. Add a recover middleware that turns it into a 500 with a request id, and never use panics to signal expected errors.

9. Doing work after the client left. A handler that ignores r.Context() keeps running a slow query for a disconnected client. Pass the context to every database and HTTP call (context cancellation).

10. Goroutines that outlive the request. Starting go sendEmail(...) inside a handler with the request's context means the email is cancelled the moment the handler returns. Use context.WithoutCancel or a proper queue (notification service).

Mistakes in the request

11. Unbounded bodies. json.NewDecoder(r.Body).Decode(&v) reads as much as the client sends. Wrap the body in http.MaxBytesReader (input limits).

12. Trusting unknown fields. Decoding into your model struct lets clients set fields you never intended (mass assignment). Decode into a request type with DisallowUnknownFields.

13. Trusting ids. A handler that loads /orders/{id} and never asks whose order it is (BOLA).

14. Parsing query parameters leniently. strconv.Atoi errors ignored, so ?limit=abc becomes 0 and returns everything or nothing. Reject bad values with 400 and cap the allowed range.

15. Using http.DefaultClient for outgoing calls. It has no timeout. A hung upstream hangs your handler forever. Build a client with timeouts (HTTP client timeouts).

Mistakes in design

16. Verbs in URLs and the wrong method. POST /getUser, or GET /orders/delete?id=3. A GET that changes state is fetched by crawlers and link preview bots. Use nouns for resources and the method for the action.

17. Inconsistent shapes. One endpoint returns {"data": [...]}, the next a bare array, a third {"results": ...}. Pick one envelope and one error shape and enforce them with shared helpers.

18. IDs, dates and money as the wrong types. Integers above 2^53 as JSON numbers, dates in local time without an offset, money as floats. Use strings for large ids, RFC 3339 UTC for timestamps and integer minor units for money.

19. No pagination until it hurts. A list endpoint that returns everything works for a hundred rows. Add limits and cursors on day one (pagination).

20. No versioning or deprecation plan. The first breaking change arrives eventually (versioning, deprecation).

A short review checklist

When reviewing a new handler, read it once for each of these questions, in this order. It takes about two minutes and catches most of the list above.

1. Is the status code written before any body byte, and is every error branch ended with a return?
2. Does every response set a content type, and is it the right one for success and for errors?
3. Is the body size limited, and are unknown fields rejected?
4. Does every database or outgoing call receive the request context, and does every outgoing client have a timeout?
5. Is every object loaded with the caller's identity in the query?
6. Could any slice or map in the response be nil, and does it matter to a client?
7. Is there a test for the unhappy paths, not just for the 200?

The point of a checklist is not to replace judgement. It removes the need to remember. Reviewers are tired, diffs are long, and the mistake in line 40 looks exactly like the correct code in line 41. Questions that you ask in the same order every time find the missing return more reliably than reading harder.

Keep the list in the repository, next to the contributing guide, and add to it when an incident teaches you something. A mistake that has caused one outage deserves a line; a mistake that has caused two deserves a lint rule or a helper that makes the wrong version impossible to write. The cheapest fixes in this article are the helpers, because they change the default for everyone who writes the next handler.

Making these less likely

Most of this list disappears with a few shared pieces: a respondJSON helper that sets the content type, status and body and handles encoding errors; a problem helper for errors; a decode helper that applies the size limit and strict decoding; and a handler test table that checks status, content type and body for each path (handler tests). Run go vet and staticcheck in CI, and add a review checklist with the five or six items your team has actually hit. The net/http documentation for ResponseWriter states the rules about headers and status that mistakes 1, 2 and 7 break, and RFC 9110 defines the status code semantics behind mistake 6.