HarperFast / HarperFast/harper

ContentTypeHandler type omits Readable from serializeStream and declares deserialize backwards

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

Description

### Bug Summary

`ContentTypeHandler.serializeStream` is declared as returning `Buffer | string`, which omits `Readable` — the type every built-in streaming handler actually returns. A typed custom handler that streams is rejected by the compiler despite being the documented, tested behavior. `deserialize` is also declared backwards.

### Where

`server/Server.ts:100-105` (at `1e4c1636f`):

```ts
export interface ContentTypeHandler {
serialize(data: any): Buffer | string;
serializeStream(data: any): Buffer | string;
deserialize(data: any): Buffer | string;
q: number;
}
```

Two of the four members disagree with the runtime.

### `serializeStream` — omits every streaming return

Built-ins in `server/serverHelpers/contentTypes.ts` return streams, not `Buffer | string`:

| handler | returns |
| --- | --- |
| `application/json`, `*/*` | `JSONStream extends Readable` (`JSONStream.ts:12-16`) |
| `application/cbor` | `new EncoderStream(...).end(data)` |
| `application/x-msgpack` | `Readable.from(encodeIter(...))` for non-array iterables; `pack(data)` (a `Buffer`) for arrays |
| `text/csv` | `toCsvStream(...)` — a piped Transform |
| `text/plain`, ndjson, `text/event-stream` | `Readable.from(...)` |

`server/http.ts:772-788` accepts more still, applied to whatever comes back: a web `ReadableStream` (via `Readable.fromWeb()`), any iterable or async iterable (via `Readable.from()`), anything with `.pipe`, or a thenable. The iterable path is covered end to end — `unitTests/testApp/resources.js:258-269` registers `serializeStream` as a generator and an async generator, asserted over HTTP by `unitTests/apiTests/basicREST-test.mjs:587-611`.

So the real union is roughly `Readable | Iterable | AsyncIterable | Buffer | string`, and the declaration admits only the last two.

### `deserialize` — declared backwards

Declared `(data: any) => Buffer | string`, but a deserializer takes a `Buffer` and returns parsed data. `contentTypes.ts:26` has the accurate shape (`(data: Buffer) => unknown`), `getDeserializer` does `streamToBuffer(stream).then(deserialize)` (`:590`), MQTT passes `packet.payload` (`server/mqtt.ts:515`), and the `text/plain` handler calls `data.toString()` itself (`:91-93`). Harper always passes a `Buffer`; implementations return objects.

### Expected Behavior

The interface should describe what handlers may return and what Harper accepts, so a typed custom handler compiles when it follows documented behavior.

### Suggested fix

Widen `serializeStream` to the union Harper actually accepts, and correct `deserialize`'s direction:

```ts
export interface ContentTypeHandler {
serialize(data: any): Buffer | Uint8Array | string | Readable;
serializeStream(data: any): Readable | Iterable | AsyncIterable | Buffer | string;
deserialize(data: Buffer): unknown;
q: number;
}
```

`serialize` is also narrower than reality — the built-in `text/csv` non-streaming `serialize` (`contentTypes.ts:77-81`) returns `toCsvStream(...)`, a stream.

Whether to keep `Buffer | string` in `serializeStream`'s union depends on HarperFast/harper#2421: if `serialize()` normalizes non-stream results before compressing, the union is safe to advertise as-is; without that fix, a `Buffer` return crashes on `Accept-Encoding: br` with a non-zero `compressionThreshold`. Worth deciding the two together.

### Notes

Surfaced while correcting `serializeStream`'s documented return type in HarperFast/documentation#655 — the docs had it as `ReadableStream`, then as `Readable`, and reviewers correctly pushed back both times. Getting this declaration right is what would let the docs stop guessing.

sent with Claude Opus 5

Contributor guide

Open the contributing guide

Research direction

Start at ContentTypeHandler in server/Server.ts and compare its declarations with the handlers in server/serverHelpers/contentTypes.ts and the accepted values in server/http.ts. Run the generator and async-generator coverage in unitTests/apiTests/basicREST-test.mjs, checking unitTests/testApp/resources.js for setup. Done means the interface matches documented runtime behavior and the related HarperFast/harper#2421 interaction is resolved.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.