HarperFast / HarperFast/harper

serialize() pipes a non-stream serializeStream() result into the Brotli compressor (TypeError: stream.pipe is not a function)

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

Description

### Bug Summary

`serialize()` pipes whatever `serializeStream()` returns straight into the Brotli compressor. A content-type handler that returns a `Buffer` or string instead of a stream — which the built-in `application/x-msgpack` handler does for a plain array — throws `TypeError: stream.pipe is not a function` and fails the request.

### Where

`server/serverHelpers/contentTypes.ts` (at `1e4c1636f`):

- `:373` — `let canCompress = COMPRESSION_THRESHOLD && request.headers.asObject?.['accept-encoding']?.includes('br');`
- `:410` — `let stream = serializer.serializer.serializeStream(responseData, responseObject);`
- `:413` — `stream = stream.pipe(createBrotliCompress({ ... }))` — unconditional when `canCompress`

The dispatch gate at `:393-398` selects `serializeStream` whenever the body is `typeof 'object'` and has `Symbol.iterator` or `Symbol.asyncIterator`. **A plain array satisfies that.** The `application/x-msgpack` handler at `:61-67` then guards with `&& !Array.isArray(data)` and falls through to `return pack(data)` — a `Buffer`, which has no `.pipe`.

### Steps to Reproduce

1. Set a non-zero `http.compressionThreshold` in `harper-config.yaml` (see the note on defaults below — the shipped value is `0`, which is why this is currently latent).
2. Export a table or resource whose response body is a plain array.
3. Request it with both headers:
```
Accept: application/x-msgpack
Accept-Encoding: br
```
4. Observed: `TypeError: stream.pipe is not a function`.

### Expected Behavior

A handler that returns a non-stream result from `serializeStream` should still produce a correct response, compressed or not. The return contract should not depend on a request header plus a config value.

### Suggested fix

Normalize before the compression branch, so the contract is header- and config-independent:

```js
let stream = serializer.serializer.serializeStream(responseData, responseObject);
if (canCompress) {
if (typeof stream?.pipe !== 'function') stream = Readable.from([stream]);
responseObject.headers.set('Content-Encoding', 'br');
stream = stream.pipe(createBrotliCompress({ ... }));
```

`Readable.from` special-cases string and `Buffer`, pushing the whole value as a single chunk, so this does not change the emitted bytes.

An alternative is to keep `serializeStream`'s contract stream-only and have the built-in msgpack handler wrap its array result. That is a smaller change but leaves every third-party handler exposed to the same trap, so normalizing in `serialize()` seems better.

### Why this is latent right now, and why that matters for sequencing

`static/defaultConfig.yaml:4` ships `http.compressionThreshold: 0`, and `canCompress` is `COMPRESSION_THRESHOLD && ...`, so a stock install never enters the compression branch at all. The crash needs a non-default threshold **plus** `Accept-Encoding: br` **plus** a non-stream return.

That matters because harper's own `config-root.schema.json:25-31` describes the default as `1200`, and its `examples` block at `:774` shows `1200`. Harper's documentation repeats `1200` in six places (tracked in HarperFast/documentation#656). **If the shipped default is "corrected" to a non-zero value before this pipe path is fixed, this becomes reachable on the same day** — including for ordinary browser clients, which all send `Accept-Encoding: br`. Worth landing this first regardless of how the default question is resolved.

### Notes

Also note the streaming compression branch sits inside the same `canCompress` gate, so `compressionThreshold: 0` disables compression for streaming responses too — the docs currently claim streaming is compressed "regardless of this setting", which is also being corrected in documentation#656.

Surfaced while documenting `serializeStream`'s return contract in HarperFast/documentation#655. Related: the `ContentTypeHandler` type declaration omits `Readable` entirely — filed separately.

sent with Claude Opus 5

Contributor guide

Open the contributing guide

Research direction

Start in server/serverHelpers/contentTypes.ts, reading the serialize() dispatch around lines 393-413 and the application/x-msgpack handler around lines 61-67. Reproduce with a non-zero compressionThreshold, Accept: application/x-msgpack, and Accept-Encoding: br; done means a plain-array response no longer fails when the handler returns a Buffer or string and the response remains correctly compressed.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.