fastify / fastify/fastify-reply-from
onResponse hook types its response argument as an outgoing ServerResponse, but receives { statusCode, headers, stream }
- 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
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