HarperFast / HarperFast/harper

SSE: mid-stream `yield null`/`undefined` tears down the whole stream — unguarded deref in serialize(); uWS delivers the crash as a clean 200

Open
#2,389 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
89
Forks
10
Avg merge
2d 6h
Merged PRs (30d)
200

Description

## What happens

A custom Resource that `yield`s a bare `null` or `undefined` mid-stream into an SSE response kills the **entire stream**: `serialize()` throws a TypeError, the producer generator is torn down (confirmed via generator `finally` counters), and the client sees an abnormal socket close. On the uWS glue it is worse: the crash is delivered as a clean, well-formed, successfully-terminated HTTP 200 (correct terminal chunk, peer FIN), so the consumer has no signal anything failed.

## Mechanism (on `origin/main` @ e16d9616)

`server/serverHelpers/contentTypes.ts:140`:

```ts
if (message.acknowledge) message.acknowledge();
```

guards the **property but not the object** — a `null`/`undefined` message throws `TypeError: Cannot read properties of null (reading 'acknowledge')` before any of the value gates run. PR #2096 (closed #2026, the falsy-drop sibling in the same eight lines) starts one line below this deref and never touches it. The upstream `transformIterable()` helper (`contentTypes.ts:626`) only special-cases the *terminal* `{value: undefined, done: true}` sentinel (the #1628 fix); a real mid-stream `yield null` passes straight through to `serialize()`.

Blast radius is bounded to custom Resources that build explicit SSE envelopes (`resources/models/openaiStream.ts`, `components/mcp/*` are in-tree examples of the pattern); real `Table.subscribe()` change-feeds, NDJSON, and plain JSON are unaffected (verified controls).

## Repro

Custom Resource whose async iterator yields `{...}, null, {...}` under `Accept: text/event-stream`. Node path: TypeError + abnormal close after the first event. uWS path: clean 200 with the tail silently missing.

## Suggested fix + test matrix

Null-guard the message object at the top of the serialize loop (skip or error loudly — either is better than tearing down the producer). The fix PR's test matrix should include an explicit `yield null` / `yield undefined` arm on both Node and uWS variants — `integrationTests/server/stream-error-contract.test.ts` (QA-890, PR #2070) already pins the general Node-vs-uWS mid-stream-throw divergence and is the natural home for that arm.

## Existing work checked

#2026/#2096 (falsy-drop, fixed — same lines, different defect), #1628/#1763 (terminal-sentinel and throw predecessors, different code path — confirmed by reading their fixes), #1653 (SSE epic, no specific issue for this), #2329 (writeHead timing on a different path). Unit tests have no null-message case.

## What this does not prove

Whether a mid-stream `null` is ever a *legitimate* value an app intends to send (the fix decides skip-vs-error); WebSocket transport unverified.

---
*From dispatch QA findings F-278 + F-280 (uWS delivery arm), verified against harper origin/main e16d9616, 2026-08-29.*

Contributor guide

Open the contributing guide

Research direction

Start in server/serverHelpers/contentTypes.ts at the serialize loop and read integrationTests/server/stream-error-contract.test.ts, which covers the Node-versus-uWS mid-stream error contract. Add coverage for explicit null and undefined yields on both transport variants, then verify the producer is not unexpectedly torn down and the uWS response does not report a clean successful completion.

Written by the indexing model from the issue text.

Assessment

Tech stack
node.js, typescript
Domain
api, backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
74/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.