renderTemplate streams to the ResponseWriter, so a mid-render template error ships a partial page #123

Closed
opened 2026-08-11 15:20:53 +02:00 by clawbot · 1 comment
Collaborator

Root cause behind a defect found in the review of #116, and worth fixing independently of it.

renderTemplate executes straight into the http.ResponseWriter. A template error part-way through therefore leaves earlier output already written and the response committed — the handler cannot then set a 500, because the header is long gone. The client gets a truncated page, typically with a 200.

It also silently weakens tests: a handler test can assert against content from the top of a page that aborted half way down and never notice. That is exactly what happened in PR 116 — three assertions passed against the flushed prefix of a page that failed at {{.Webhook.RetentionLabel}}.

Standard fix: execute into a bytes.Buffer, and only on success set the content type and copy the buffer to the ResponseWriter. On error, nothing has been written, so a real 500 can still be served.

Definition of done

  • A template that errors mid-render produces a 500 and NO partial body.
  • Successful renders are byte-identical to today.
  • A test that renders a deliberately failing template and asserts on both the status and the empty body.

Implementation requirements

  • Consider the memory trade-off and say why it is acceptable here: these pages are small, and buffering is the conventional choice for exactly this reason.
  • Branch from next, PR based on next, single commit, title ending (closes #N).
  • Gate on make check plus the Docker lint path with the cache defeated.
Root cause behind a defect found in the review of https://git.eeqj.de/sneak/webhooker/pulls/116, and worth fixing independently of it. `renderTemplate` executes straight into the `http.ResponseWriter`. A template error part-way through therefore leaves earlier output already written and the response committed — the handler cannot then set a 500, because the header is long gone. The client gets a truncated page, typically with a 200. It also silently weakens tests: a handler test can assert against content from the top of a page that aborted half way down and never notice. That is exactly what happened in PR 116 — three assertions passed against the flushed prefix of a page that failed at `{{.Webhook.RetentionLabel}}`. Standard fix: execute into a `bytes.Buffer`, and only on success set the content type and copy the buffer to the `ResponseWriter`. On error, nothing has been written, so a real 500 can still be served. ## Definition of done - A template that errors mid-render produces a 500 and NO partial body. - Successful renders are byte-identical to today. - A test that renders a deliberately failing template and asserts on both the status and the empty body. ## Implementation requirements - Consider the memory trade-off and say why it is acceptable here: these pages are small, and buffering is the conventional choice for exactly this reason. - Branch from `next`, PR based on `next`, single commit, title ending ` (closes #N)`. - Gate on `make check` plus the Docker lint path with the cache defeated.
Author
Collaborator

Implemented in #131 (base next). executeTemplate renders into a bytes.Buffer and writes the content type and body only after a successful execute.

Verification, detailed in the PR body: the new test fails against the pre-fix streaming renderer (200 with PARTIAL PAGE CONTENTInternal server error), and a ten-case capture across all seven page templates diffs byte-identical before and after. Residual on the five paths that set a 4xx before rendering is filed as #128.

Implemented in https://git.eeqj.de/sneak/webhooker/pulls/131 (base `next`). `executeTemplate` renders into a `bytes.Buffer` and writes the content type and body only after a successful execute. Verification, detailed in the PR body: the new test fails against the pre-fix streaming renderer (200 with `PARTIAL PAGE CONTENTInternal server error`), and a ten-case capture across all seven page templates diffs byte-identical before and after. Residual on the five paths that set a 4xx before rendering is filed as https://git.eeqj.de/sneak/webhooker/issues/128.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/webhooker#123