Merge next into master (#371)

* feat: pass full response object to onResponse hook Before this change, the onResponse hook was only given the response stream to work with. This was useful if you wanted to transform the response body on a chunk by chunk basis, but did not allow inspecting the status code or headers of the response! If you wanted to serve a custom error page or similar, you couldn't get at the actual `res` object in question. So, this changes the `onResponse` hook to instead pass the whole response instead of just the stream. Now, users can inspect the status code as well. * Update for v5 (#359) * update except tap * tap18 * Revert "tap18" This reverts commit 230214e. * update undici * update multipart * update * update tests --------- Signed-off-by: Gürgün Dayıoğlu <hey@gurgun.day> --------- Signed-off-by: Gürgün Dayıoğlu <hey@gurgun.day> Co-authored-by: Harry Brundage <harry.brundage@gmail.com> Co-authored-by: Gürgün Dayıoğlu <hey@gurgun.day>
fastify · Jun 15, 2024 · 9dd31ef · 9dd31ef
1 parent d038027
commit 9dd31ef
Show file tree

Hide file tree

Showing 24 changed files with 111 additions and 120 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -17,7 +17,7 @@ on:
 
 jobs:
   test:
-    uses: fastify/workflows/.github/workflows/plugins-ci.yml@v3
+    uses: fastify/workflows/.github/workflows/plugins-ci.yml@v4.1.0
     with:
       license-check: true
       lint: true
diff --git a/.taprc b/.taprc
@@ -1,11 +1,3 @@
-ts: false
-jsx: false
-flow: false
-
-functions: 97
-lines: 96
-statements: 96
-branches: 96
-
+disable-coverage: true
 files:
   - test/**/*.test.js
diff --git a/README.md b/README.md
@@ -344,10 +344,11 @@ the request or response being sent or received to/from the source.
 
 **Note: If `base` is specified in plugin options, the `source` here should not override the host/origin.**
 
-#### `onResponse(request, reply, res)`
+#### `onResponse(request, reply, response)`
 
-Called when a HTTP response is received from the source.
-The default behavior is `reply.send(res)`, which will be disabled if the
+Called when a HTTP response is received from the source. Passed the original source `request`, the in-progress reply to the source as `reply`, and the ongoing `response` from the upstream server.
+
+The default behavior is `reply.send(response.stream)`, which will be disabled if the
 option is specified.
 
 When replying with a body of a different length it is necessary to remove
@@ -362,6 +363,8 @@ the `content-length` header.
 }
 ```
 
+**Note**: `onResponse` is called after headers have already been sent. If you want to modify response headers, use the `rewriteHeaders` hook.
+
 #### `onError(reply, error)`
 
 Called when a HTTP response is received with error from the source.

diff --git a/index.js b/index.js
@@ -201,7 +201,7 @@ const fastifyReplyFrom = fp(function from (fastify, opts, next) {
       }
       this.code(res.statusCode)
       if (onResponse) {
-        onResponse(this.request, this, res.stream)
+        onResponse(this.request, this, res)
       } else {
         this.send(res.stream)
       }
@@ -267,9 +267,7 @@ function onErrorDefault (reply, { error }) {
 }
 
 function isFastifyMultipartRegistered (fastify) {
-  // TODO: remove fastify.hasContentTypeParser('multipart') in next major
-  // It is used to be compatible with @fastify/multipart@<=7.3.0
-  return (fastify.hasContentTypeParser('multipart') || fastify.hasContentTypeParser('multipart/form-data')) && fastify.hasRequestDecorator('multipart')
+  return fastify.hasContentTypeParser('multipart/form-data')
 }
 
 function createRequestRetry (requestImpl, reply, retryHandler) {

diff --git a/package.json b/package.json
@@ -30,35 +30,35 @@
   "homepage": "https://github.com/fastify/fastify-reply-from#readme",
   "devDependencies": {
     "@fastify/formbody": "^7.4.0",
-    "@fastify/multipart": "^7.4.0",
-    "@fastify/pre-commit": "^2.0.2",
-    "@sinonjs/fake-timers": "^11.0.0",
-    "@types/node": "^20.1.4",
-    "@types/tap": "^15.0.7",
-    "fastify": "^4.0.2",
+    "@fastify/multipart": "^8.2.0",
+    "@fastify/pre-commit": "^2.1.0",
+    "@sinonjs/fake-timers": "^11.2.2",
+    "@types/node": "^20.11.30",
+    "@types/tap": "^15.0.11",
+    "fastify": "^4.26.2",
     "form-data": "^4.0.0",
-    "got": "^11.8.2",
+    "got": "^11.8.6",
     "h2url": "^0.2.0",
-    "msgpack5": "^6.0.1",
-    "nock": "^13.2.6",
+    "msgpack5": "^6.0.2",
+    "nock": "^13.5.4",
     "proxy": "^2.1.1",
     "proxyquire": "^2.1.3",
-    "semver": "^7.5.1",
+    "semver": "^7.6.0",
     "simple-get": "^4.0.1",
     "snazzy": "^9.0.0",
-    "split2": "^4.1.0",
-    "standard": "^17.0.0",
-    "tap": "^16.2.0",
+    "split2": "^4.2.0",
+    "standard": "^17.1.0",
+    "tap": "^18.7.2",
     "tsd": "^0.31.0"
   },
   "dependencies": {
-    "@fastify/error": "^3.0.0",
+    "@fastify/error": "^3.4.1",
     "end-of-stream": "^1.4.4",
     "fast-content-type-parse": "^1.1.0",
-    "fast-querystring": "^1.0.0",
-    "fastify-plugin": "^4.0.0",
+    "fast-querystring": "^1.1.2",
+    "fastify-plugin": "^4.5.1",
     "toad-cache": "^3.7.0",
-    "undici": "^5.19.1"
+    "undici": "^6.11.1"
   },
   "pre-commit": [
     "lint",

diff --git a/test/full-delete-http2.test.js b/test/full-delete-http2.test.js
@@ -47,4 +47,6 @@ test('http -> http2', async function (t) {
   t.equal(headers['x-my-header'], 'hello!')
   t.match(headers['content-type'], /application\/json/)
   t.same(body, { hello: 'world' })
+  instance.close()
+  target.close()
 })
diff --git a/test/full-post-http2.test.js b/test/full-post-http2.test.js
@@ -46,4 +46,6 @@ test('http -> http2', async function (t) {
   t.equal(headers['x-my-header'], 'hello!')
   t.match(headers['content-type'], /application\/json/)
   t.same(body, { hello: 'world' })
+  instance.close()
+  target.close()
 })
diff --git a/test/full-querystring-rewrite-option-function-request.test.js b/test/full-querystring-rewrite-option-function-request.test.js
@@ -51,6 +51,8 @@ target.listen({ port: 0 }, (err) => {
       t.equal(res.headers['x-my-header'], 'hello!')
       t.equal(res.statusCode, 205)
       t.equal(data.toString(), 'hello world')
+      instance.close()
+      target.close()
     })
   })
 })
diff --git a/test/http-http2.test.js b/test/http-http2.test.js
@@ -43,6 +43,8 @@ test('http -> http2', async (t) => {
     t.equal(err.response.headers['x-my-header'], 'hello!')
     t.match(err.response.headers['content-type'], /application\/json/)
     t.same(JSON.parse(err.response.body), { hello: 'world' })
+    instance.close()
+    target.close()
     return
   }
 

diff --git a/test/http-timeout.test.js b/test/http-timeout.test.js
@@ -9,8 +9,6 @@ const FakeTimers = require('@sinonjs/fake-timers')
 const clock = FakeTimers.createClock()
 
 test('http request timeout', async (t) => {
-  t.autoend(false)
-
   const target = Fastify()
   t.teardown(target.close.bind(target))
 

diff --git a/test/http2-http2.test.js b/test/http2-http2.test.js
@@ -54,4 +54,6 @@ t.test('http2 -> http2', async (t) => {
   t.equal(headers['x-my-header'], 'hello!')
   t.match(headers['content-type'], /application\/json/)
   t.same(JSON.parse(body), { hello: 'world' })
+  instance.close()
+  target.close()
 })
diff --git a/test/http2-invalid-base.test.js b/test/http2-invalid-base.test.js
@@ -7,7 +7,7 @@ const From = require('../index')
 test('http2 invalid base', async (t) => {
   const instance = Fastify()
 
-  await t.rejects(instance.register(From, {
+  await t.rejects(async () => instance.register(From, {
     http2: { requestTimeout: 100 }
   }), new Error('Option base is required when http2 is true'))
 })
diff --git a/test/http2-timeout.test.js b/test/http2-timeout.test.js
@@ -122,4 +122,6 @@ test('http2 sse removes request and session timeout test', async (t) => {
 
   const { statusCode } = await got.get(`http://localhost:${instance.server.address().port}/`, { retry: 0 })
   t.equal(statusCode, 200)
+  instance.close()
+  target.close()
 })
diff --git a/test/http2-unix-socket.test.js b/test/http2-unix-socket.test.js
@@ -9,7 +9,7 @@ test('throw an error if http2 is used with a Unix socket destination', async t =
 
   const instance = Fastify()
 
-  await t.rejects(instance.register(From, {
+  await t.rejects(async () => instance.register(From, {
     base: 'unix+http://localhost:1337',
     http2: { requestTimeout: 100 }
   }), new Error('Unix socket destination is not supported when http2 is true'))

diff --git a/test/on-error.test.js b/test/on-error.test.js
@@ -8,21 +8,19 @@ const FakeTimers = require('@sinonjs/fake-timers')
 
 const clock = FakeTimers.createClock()
 
-t.autoend(false)
+t.test('on-error', async (t) => {
+  const target = Fastify()
+  t.teardown(target.close.bind(target))
 
-const target = Fastify()
-t.teardown(target.close.bind(target))
+  target.get('/', (request, reply) => {
+    t.pass('request arrives')
 
-target.get('/', (request, reply) => {
-  t.pass('request arrives')
-
-  clock.setTimeout(() => {
-    reply.status(200).send('hello world')
-    t.end()
-  }, 1000)
-})
+    clock.setTimeout(() => {
+      reply.status(200).send('hello world')
+      t.end()
+    }, 1000)
+  })
 
-async function main () {
   await target.listen({ port: 0 })
 
   const instance = Fastify()
@@ -63,6 +61,4 @@ async function main () {
   }
 
   t.fail()
-}
-
-main()
+})
diff --git a/test/onResponse.test.js b/test/onResponse.test.js
@@ -9,7 +9,7 @@ const get = require('simple-get').concat
 const instance = Fastify()
 instance.register(From)
 
-t.plan(8)
+t.plan(9)
 t.teardown(instance.close.bind(instance))
 
 const target = http.createServer((req, res) => {
@@ -22,8 +22,9 @@ const target = http.createServer((req, res) => {
 instance.get('/', (request1, reply) => {
   reply.from(`http://localhost:${target.address().port}`, {
     onResponse: (request2, reply, res) => {
+      t.equal(res.statusCode, 200)
       t.equal(request1.raw, request2.raw)
-      reply.send(res)
+      reply.send(res.stream)
     }
   })
 })

diff --git a/test/transform-body.test.js b/test/transform-body.test.js
@@ -26,7 +26,7 @@ instance.get('/', (request, reply) => {
   reply.from(`http://localhost:${target.address().port}`, {
     onResponse: (request, reply, res) => {
       reply.send(
-        res.pipe(
+        res.stream.pipe(
           new Transform({
             transform: function (chunk, enc, cb) {
               this.push(chunk.toString().toUpperCase())

diff --git a/test/undici-agent.test.js b/test/undici-agent.test.js
@@ -31,7 +31,7 @@ target.listen({ port: 0 }, err => {
   const From = proxyquire('..', {
     './lib/request.js': proxyquire('../lib/request.js', {
       undici: proxyquire('undici', {
-        './lib/agent': proxyquire('undici/lib/agent.js', {
+        './lib/dispatcher/agent': proxyquire('undici/lib/dispatcher/agent.js', {
           './pool': class Pool extends undici.Pool {
             constructor (url, options) {
               super(url, options)

diff --git a/test/undici-connect-timeout.test.js b/test/undici-connect-timeout.test.js
@@ -7,18 +7,16 @@ const Fastify = require('fastify')
 const From = require('..')
 const got = require('got')
 
-t.autoend(false)
-
+t.test('undici connect timeout', async (t) => {
 // never connect
-net.connect = function (options) {
-  return new net.Socket(options)
-}
+  net.connect = function (options) {
+    return new net.Socket(options)
+  }
 
-const target = http.createServer((req, res) => {
-  t.fail('target never called')
-})
+  const target = http.createServer((req, res) => {
+    t.fail('target never called')
+  })
 
-async function main () {
   t.plan(2)
   await target.listen({ port: 0 })
 
@@ -53,6 +51,4 @@ async function main () {
   }
 
   t.fail()
-}
-
-main()
+})
diff --git a/test/undici-proxy-agent.test.js b/test/undici-proxy-agent.test.js
@@ -56,6 +56,8 @@ for (const [description, format] of Object.entries(configFormat)) {
           t.same(res.statusCode, 200)
           t.match(JSON.parse(data.toString()), { hello: 'world' })
           resolve()
+          instance.close()
+          target.close()
         })
       })
     })

diff --git a/test/undici-timeout-body-partial.test.js b/test/undici-timeout-body-partial.test.js
@@ -9,23 +9,21 @@ const FakeTimers = require('@sinonjs/fake-timers')
 
 const clock = FakeTimers.createClock()
 
-t.autoend(false)
-
-const target = http.createServer((req, res) => {
-  t.pass('request proxied')
-  req.on('data', () => undefined)
-  req.on('end', () => {
-    res.writeHead(200)
-    res.flushHeaders()
-    res.write('test')
-    clock.setTimeout(() => {
-      res.end()
-      t.end()
-    }, 1000)
+t.test('undici body timeout', async (t) => {
+  const target = http.createServer((req, res) => {
+    t.pass('request proxied')
+    req.on('data', () => undefined)
+    req.on('end', () => {
+      res.writeHead(200)
+      res.flushHeaders()
+      res.write('test')
+      clock.setTimeout(() => {
+        res.end()
+        t.end()
+      }, 1000)
+    })
   })
-})
 
-async function main () {
   await target.listen({ port: 0 })
 
   const instance = Fastify()
@@ -55,6 +53,4 @@ async function main () {
   }
 
   t.fail()
-}
-
-main()
+})
diff --git a/test/undici-timeout-body.test.js b/test/undici-timeout-body.test.js
@@ -9,21 +9,19 @@ const FakeTimers = require('@sinonjs/fake-timers')
 
 const clock = FakeTimers.createClock()
 
-t.autoend(false)
-
-const target = http.createServer((req, res) => {
-  t.pass('request proxied')
-  req.on('data', () => undefined)
-  req.on('end', () => {
-    res.flushHeaders()
-    clock.setTimeout(() => {
-      res.end()
-      t.end()
-    }, 1000)
+t.test('undici body timeout', async (t) => {
+  const target = http.createServer((req, res) => {
+    t.pass('request proxied')
+    req.on('data', () => undefined)
+    req.on('end', () => {
+      res.flushHeaders()
+      clock.setTimeout(() => {
+        res.end()
+        t.end()
+      }, 1000)
+    })
   })
-})
 
-async function main () {
   await target.listen({ port: 0 })
 
   const instance = Fastify()
@@ -58,6 +56,4 @@ async function main () {
   }
 
   t.fail()
-}
-
-main()
+})