Worked review 1: HTTP JSON API handler

This is a small task-list HTTP API: POST /tasks creates a task, GET /tasks/{id} fetches one. It is written in the confident, plausible style an AI assistant produces on the first try — it compiles, go vet says nothing, and it runs. Read it before running it. It has 5 real bugs.

The program

Run ▸ Open in Playground

package main

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

type Task struct {
  ID   int    `json:"id"`
  Text string `json:"text"`
}

type Store struct {
  tasks  map[int]Task
  nextID int
}

func NewStore() *Store {
  return &Store{tasks: map[int]Task{}, nextID: 1}
}

func (s Store) Add(text string) Task {
  t := Task{ID: s.nextID, Text: text}
  s.tasks[t.ID] = t
  s.nextID++
  return t
}

func (s *Store) Get(id int) (Task, bool) {
  t, ok := s.tasks[id]
  return t, ok
}

func newMux(store *Store) *http.ServeMux {
  mux := http.NewServeMux()
  mux.HandleFunc("POST /tasks", func(w http.ResponseWriter, r *http.Request) {
    var body struct {
      Text string `json:"text"`
    }
    json.NewDecoder(r.Body).Decode(&body)
    t := store.Add(body.Text)
    json.NewEncoder(w).Encode(t)
    w.WriteHeader(http.StatusCreated)
  })
  mux.HandleFunc("GET /tasks/{id}", func(w http.ResponseWriter, r *http.Request) {
    id, _ := strconv.Atoi(r.PathValue("id"))
    t, ok := store.Get(id)
    if !ok {
      w.WriteHeader(http.StatusNotFound)
    }
    json.NewEncoder(w).Encode(t)
  })
  return mux
}

func main() {
  store := NewStore()
  ts := httptest.NewServer(newMux(store))
  defer ts.Close()

  for _, text := range []string{"buy milk", "write report"} {
    payload := strings.NewReader(fmt.Sprintf(`{"text":%q}`, text))
    resp, _ := http.Post(ts.URL+"/tasks", "application/json", payload)
    body, _ := io.ReadAll(resp.Body)
    fmt.Printf("POST /tasks status=%d body=%s\n", resp.StatusCode, body)
  }

  resp, _ := http.Post(ts.URL+"/tasks", "application/json", strings.NewReader("not json"))
  body, _ := io.ReadAll(resp.Body)
  fmt.Printf("POST /tasks (bad json) status=%d body=%s\n", resp.StatusCode, body)

  resp, _ = http.Get(ts.URL + "/tasks/abc")
  body, _ = io.ReadAll(resp.Body)
  fmt.Printf("GET /tasks/abc status=%d body=%s\n", resp.StatusCode, body)

  resp, _ = http.Get(ts.URL + "/tasks/999")
  body, _ = io.ReadAll(resp.Body)
  fmt.Printf("GET /tasks/999 status=%d body=%s\n", resp.StatusCode, body)
}

Find the bugs

Run it. Every response comes back with status=200, one task’s id never changes, and a 404 has a body attached to it. None of that is intentional. Before opening the answer below, try to name at least three of the five bugs and which line causes each.

Reveal the 5 bugs

1. func (s Store) Add(...) has a value receiver. Add mutates s.nextID and writes into s.tasks — but s is a full copy of Store on every call. The map write survives (a map value is a shared reference), but s.nextID++ only increments the copy; the real Store.nextID never moves past 1. Every task created gets "id":1, silently colliding in the map. Real output: both buy milk and write report come back as {"id":1,...} — the second call overwrote the first in store.tasks.
2. json.NewDecoder(r.Body).Decode(&body)’s error is ignored. POSTing "not json" doesn’t fail — body.Text just stays "", and the handler happily creates a task with empty text and returns 200.
3. w.WriteHeader(http.StatusCreated) is called after json.NewEncoder(w).Encode(t). Encode calls Write internally, and the first Write on a ResponseWriter implicitly sends a 200 status if WriteHeader hasn’t been called yet. By the time WriteHeader(201) runs, the response is already sent — go run logs http: superfluous response.WriteHeader call and the client sees 200, never 201.
4. The 404 branch does not return. if !ok { w.WriteHeader(http.StatusNotFound) } sends the 404 header, then execution falls through to json.NewEncoder(w).Encode(t) anyway — encoding the zero-value Task{}. GET /tasks/999 comes back 404 with a body of {"id":0,"text":""}, which looks like a malformed but existing task rather than “not found.”
5. strconv.Atoi(r.PathValue("id"))’s error is ignored. GET /tasks/abc doesn’t return 400 Bad Request — id silently becomes 0, and the handler proceeds to look up task 0 as if that were a normal, valid request.

The corrected program

Run ▸ Open in Playground

package main

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

type Task struct {
  ID   int    `json:"id"`
  Text string `json:"text"`
}

type Store struct {
  mu     sync.Mutex // fixed: guards tasks/nextID against concurrent handlers
  tasks  map[int]Task
  nextID int
}

func NewStore() *Store {
  return &Store{tasks: map[int]Task{}, nextID: 1}
}

func (s *Store) Add(text string) Task { // fixed: pointer receiver — nextID actually persists
  s.mu.Lock()
  defer s.mu.Unlock()
  t := Task{ID: s.nextID, Text: text}
  s.tasks[t.ID] = t
  s.nextID++
  return t
}

func (s *Store) Get(id int) (Task, bool) {
  s.mu.Lock()
  defer s.mu.Unlock()
  t, ok := s.tasks[id]
  return t, ok
}

func newMux(store *Store) *http.ServeMux {
  mux := http.NewServeMux()
  mux.HandleFunc("POST /tasks", func(w http.ResponseWriter, r *http.Request) {
    var body struct {
      Text string `json:"text"`
    }
    if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
      http.Error(w, "invalid JSON body", http.StatusBadRequest)
      return
    }
    t := store.Add(body.Text)
    w.WriteHeader(http.StatusCreated) // fixed: header before the first Write
    _ = json.NewEncoder(w).Encode(t)
  })
  mux.HandleFunc("GET /tasks/{id}", func(w http.ResponseWriter, r *http.Request) {
    id, err := strconv.Atoi(r.PathValue("id"))
    if err != nil {
      http.Error(w, "invalid id", http.StatusBadRequest)
      return
    }
    t, ok := store.Get(id)
    if !ok {
      w.WriteHeader(http.StatusNotFound)
      return // fixed: stop here, do not also write a zero-value body
    }
    _ = json.NewEncoder(w).Encode(t)
  })
  return mux
}

func main() {
  store := NewStore()
  ts := httptest.NewServer(newMux(store))
  defer ts.Close()

  for _, text := range []string{"buy milk", "write report"} {
    payload := strings.NewReader(fmt.Sprintf(`{"text":%q}`, text))
    resp, _ := http.Post(ts.URL+"/tasks", "application/json", payload)
    body, _ := io.ReadAll(resp.Body)
    fmt.Printf("POST /tasks status=%d body=%s\n", resp.StatusCode, body)
  }

  resp, _ := http.Post(ts.URL+"/tasks", "application/json", strings.NewReader("not json"))
  body, _ := io.ReadAll(resp.Body)
  fmt.Printf("POST /tasks (bad json) status=%d body=%s\n", resp.StatusCode, body)

  resp, _ = http.Get(ts.URL + "/tasks/abc")
  body, _ = io.ReadAll(resp.Body)
  fmt.Printf("GET /tasks/abc status=%d body=%s\n", resp.StatusCode, body)

  resp, _ = http.Get(ts.URL + "/tasks/999")
  body, _ = io.ReadAll(resp.Body)
  fmt.Printf("GET /tasks/999 status=%d body=%s\n", resp.StatusCode, body)
}

Run the corrected version: IDs increment (1, then 2), a malformed body now gets 400 invalid JSON body, /tasks/abc gets 400 invalid id instead of a false 404, and /tasks/999 gets a real 404 with an empty body. go vet and golangci-lint run both report zero issues on this version — the added sync.Mutex also closes the concurrency-safety gap a pointer-receiver Store would otherwise still have under real concurrent traffic (this demo’s requests run sequentially, so the original wouldn’t have shown a race here, but production traffic would).

🟦 Value receivers on a type with mutable state are the recurring theme

Bug 1 alone explains why Store.Add needs a pointer receiver: a value receiver silently drops every mutation except the ones that go through a reference type like a map. See Methods and pattern 13 in the AI Bug Catalog.

In the buggy version, both POST requests return {"id":1,...}. What is the direct cause?

The Store's map has a maximum size of 1 Add has a value receiver, so s.nextID++ increments a copy of nextID that is discarded when Add returns The httptest server caches the first response JSON encoding always resets numeric fields to 1

Why does the buggy handler's client always see status=200 instead of 201, even though the code calls w.WriteHeader(http.StatusCreated)?

http.StatusCreated is actually the same value as 200 WriteHeader is called after Encode has already triggered an implicit 200 on the first Write httptest.NewServer overrides all status codes to 200 The client library ignores status codes above 200

GET /tasks/999 (a task that does not exist) returns 404 with body {"id":0,"text":""} in the buggy version. What single missing keyword causes the body to be sent at all?

return — without it, execution falls through to the Encode call after WriteHeader(404) break — the handler needs a labeled break continue — the loop needs to skip an iteration defer — the encode call should have been deferred

The fixed version adds a sync.Mutex to Store even though this demo's sequential requests never actually raced. Why add it anyway?

It is required by the Go compiler for any struct with a map field A real HTTP server handles requests concurrently; without a mutex, concurrent POST/GET calls would race on the shared map and nextID field even after the receiver bug is fixed It has no real benefit — it was added only for symmetry with the AI Bug Catalog sync.Mutex makes JSON encoding faster

Submit

Mark as complete