fastify / fastify/fastify-reply-from

onResponse hook types its response argument as an outgoing ServerResponse, but receives { statusCode, headers, stream }

Open
#494 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
JavaScript
Stars
166
Forks
103
Avg merge
4h 9m
Merged PRs (30d)
4

Description

### Prerequisites

- [x] I have written a descriptive issue title
- [x] I have searched existing issues to ensure the bug has not already been reported

### Fastify version

5.12.1

### Plugin version

12.6.4

### Node.js version

22.x

### Operating system

macOS

### Operating system version (i.e. 20.04, 11.3, 10)

15

### Description

The third argument to the `onResponse` hook is typed as an **outgoing** response, but at runtime it is a plain object describing an **incoming** one. The two have disjoint APIs, so the type both advertises methods that do not exist and hides the properties that do.

```ts
export type RawServerResponse = RawReplyDefaultExpression & {
stream: IncomingMessage
}
```

`RawReplyDefaultExpression` resolves to `http.ServerResponse`, which has `getHeader()` / `setHeader()` / `writeHead()` and no `headers` property. The object actually passed is built in `lib/request.js`, identically across all three transports:

```js
done(null, { statusCode: res.statusCode, headers: res.headers, stream: res }) // http/https agent
done(null, { statusCode: res.statusCode, headers: res.headers, stream: res.body }) // undici
done(null, { statusCode, headers, stream: req }) // http2
```

so it is `{ statusCode, headers, stream }` and nothing else.

The practical consequence is that reading a response header from `onResponse` — for example `location`, to handle a redirect from an upstream — does not type-check, while `res.getHeader('location')` does type-check and throws `res.getHeader is not a function` at runtime.

#383 reported the `stream` half of this and was fixed by intersecting `{ stream: IncomingMessage }` onto the type. That made `stream` reachable but left the incorrect base in place, so `headers` and `statusCode` are still wrong.

### Link to code that reproduces the bug

```js
import http from 'node:http'
import Fastify from 'fastify'
import replyFrom from '@fastify/reply-from'

const upstream = http.createServer((q, s) => {
s.writeHead(302, { location: 'https://example.com/blob' })
s.end()
})
await new Promise((r) => upstream.listen(0, '127.0.0.1', r))

const app = Fastify()
await app.register(replyFrom, { base: `http://127.0.0.1:${upstream.address().port}` })
app.get('/x', (req, reply) =>
reply.from(undefined, {
onResponse (request, rep, res) {
console.log('typeof res.getHeader :', typeof res.getHeader)
console.log('typeof res.headers :', typeof res.headers)
console.log('res.headers.location :', res.headers?.location)
console.log('own keys :', Object.keys(res))
console.log('constructor :', res.constructor?.name)
rep.send(res.stream)
}
}))
await app.ready()
await app.inject({ method: 'GET', url: '/x' })
```

Output:

```
typeof res.getHeader : undefined
typeof res.headers : object
res.headers.location : https://example.com/blob
own keys : [ 'statusCode', 'headers', 'stream' ]
constructor : Object
```

In TypeScript, `res.headers` is a compile error (`Property 'headers' does not exist`) even though it is the only way to read a response header, and `res.getHeader('location')` compiles but throws.

### Expected Behavior

`onResponse` should describe the object that is actually passed, rather than intersecting `stream` onto an unrelated outgoing-response type. Something like:

```ts
export interface FastifyReplyFromResponse {
statusCode: number
headers: IncomingHttpHeaders
stream: Readable
}
```

`Readable` rather than `IncomingMessage` because the three transports supply different stream types — `IncomingMessage` for the http/https agent, undici's `BodyReadable` for undici, and `ClientHttp2Stream` for http2 — and `Readable` is the common supertype. That was raised as an open question in #383 and this seems the least surprising resolution.

The current `RawServerResponse` export would presumably need to stay for backwards compatibility, or its removal would be a breaking change for anyone importing it.

Happy to send a PR with `tsd` assertions if a maintainer confirms the preferred shape.

Contributor guide

Open the contributing guide

Research direction

Start with the response objects assembled in lib/request.js and trace the onResponse type to RawServerResponse. Add tsd assertions for statusCode, headers, and stream across the three transports, while preserving the current RawServerResponse export for compatibility. Done means the actual response shape type-checks and the misleading outgoing-response methods no longer do.

Written by the indexing model from the issue text.

Assessment

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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.