HarperFast / HarperFast/harper
ContentTypeHandler type omits Readable from serializeStream and declares deserialize backwards
- 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
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