Skip to content

Commit 2971a9a

Browse files
committed
admin: marshal-then-write JSON responses (Gemini medium)
writeAdminJSON used to stream json.NewEncoder(w).Encode(...) directly to the ResponseWriter. If marshalling failed midway through the body (an unsupported type, a Marshaler returning an error), the 200 status header was already on the wire and the client received a truncated / malformed JSON object — unrecoverable on the SPA side. Marshal to []byte first, *then* write the status + body. An encode failure now upgrades to a clean 500 with the standard error envelope. Write failures after the status is committed remain log-only. Also documents the deliberate "materialise the full table list before paginate-and-slice" choice in handleList so future readers know the adapter's listTableNames already scans the entire metadata prefix in one shot for the SigV4 path; streaming on top would not change the adapter's memory profile.
1 parent f5d6cef commit 2971a9a

1 file changed

Lines changed: 35 additions & 6 deletions

File tree

internal/admin/dynamo_handler.go

Lines changed: 35 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,17 @@ func (h *DynamoHandler) handleList(w http.ResponseWriter, r *http.Request) {
131131
return
132132
}
133133

134+
// AdminListTables materialises the full table-name list before
135+
// paginate-and-slice. The adapter's listTableNames already
136+
// scans the entire metadata prefix in one shot for the SigV4
137+
// listTables path, so streaming here would not change the
138+
// adapter's memory profile; the dashboard's bounded `limit`
139+
// (default 100, hard max 1000) and the lex-sorted name list
140+
// keep the per-request slice small enough that this is the
141+
// pragmatic shape rather than the limiting factor. If a future
142+
// admin-cluster scale ever changes that calculus, the fix is to
143+
// teach the adapter to stream, then plumb that through here —
144+
// not to add a streaming layer on top of the materialised list.
134145
names, err := h.source.AdminListTables(r.Context())
135146
if err != nil {
136147
h.logger.LogAttrs(r.Context(), slog.LevelError, "admin dynamo list tables failed",
@@ -250,19 +261,37 @@ func paginateDynamoTableNames(names []string, startAfter string, limit int) ([]s
250261
return page, ""
251262
}
252263

253-
// writeAdminJSON is the shared 200-OK JSON writer. Encoder errors
254-
// are logged but cannot be reported to the client because the 200
255-
// header has already been flushed.
264+
// writeAdminJSON marshals `body` to a buffer first, *then* writes
265+
// status + body — never streaming an encoder directly to the
266+
// ResponseWriter. The streaming form would commit a 200 header and
267+
// then truncate mid-body if json.Marshal failed on a value deep in
268+
// the struct (an unsupported type, a Marshaler returning an error,
269+
// etc.), leaving a malformed JSON object on the wire that the SPA
270+
// has no way to recover from. Marshalling first lets us upgrade the
271+
// encode failure to a 500 with a well-formed error envelope.
256272
func writeAdminJSON(w http.ResponseWriter, ctx context.Context, logger *slog.Logger, body any) {
273+
payload, err := json.Marshal(body)
274+
if err != nil {
275+
if logger == nil {
276+
logger = slog.Default()
277+
}
278+
logger.LogAttrs(ctx, slog.LevelError, "admin response marshal failed",
279+
slog.String("error", err.Error()),
280+
)
281+
writeJSONError(w, http.StatusInternalServerError, "internal", "failed to encode response")
282+
return
283+
}
257284
w.Header().Set("Content-Type", "application/json; charset=utf-8")
258285
w.Header().Set("Cache-Control", "no-store")
259286
w.WriteHeader(http.StatusOK)
260-
if err := json.NewEncoder(w).Encode(body); err != nil {
287+
if _, werr := w.Write(payload); werr != nil {
288+
// Status is already on the wire, so we can only log. Write
289+
// failures here usually mean the client closed the connection.
261290
if logger == nil {
262291
logger = slog.Default()
263292
}
264-
logger.LogAttrs(ctx, slog.LevelWarn, "admin response encode failed",
265-
slog.String("error", err.Error()),
293+
logger.LogAttrs(ctx, slog.LevelWarn, "admin response write failed",
294+
slog.String("error", werr.Error()),
266295
)
267296
}
268297
}

0 commit comments

Comments
 (0)