feat: support async trailer (#4380)

* feat: support async trailer

* test: increase coverage

* docs: add note about error

* test: increase coverage

* docs: wording

* feat: deprecation

docs/Reference/Reply.md

@@ -234,8 +234,8 @@ as soon as possible.
 *Note: The header `Transfer-Encoding: chunked` will be added once you use the
 trailer. It is a hard requirement for using trailer in Node.js.*
 
-*Note: Currently, the computation function only supports synchronous function.
-That means `async-await` and `promise` are not supported.*
+*Note: Any error passed to `done` callback will be ignored. If you interested
+in the error, you can turn on `debug` level logging.*
 
 ```js
 reply.trailer('server-timing', function() {
@@ -246,7 +246,15 @@ const { createHash } = require('crypto')
 // trailer function also recieve two argument
 // @param {object} reply fastify reply
 // @param {string|Buffer|null} payload payload that already sent, note that it will be null when stream is sent
-reply.trailer('content-md5', function(reply, payload) {
+// @param {function} done callback to set trailer value
+reply.trailer('content-md5', function(reply, payload, done) {
+  const hash = createHash('md5')
+  hash.update(payload)
+  done(null, hash.disgest('hex'))
+})
+
+// when you prefer async-await
+reply.trailer('content-md5', async function(reply, payload) {
   const hash = createHash('md5')
   hash.update(payload)
   return hash.disgest('hex')

lib/reply.js

@@ -673,11 +673,38 @@ function sendTrailer (payload, res, reply) {
   if (reply[kReplyTrailers] === null) return
   const trailerHeaders = Object.keys(reply[kReplyTrailers])
   const trailers = {}
+  let handled = 0
   for (const trailerName of trailerHeaders) {
     if (typeof reply[kReplyTrailers][trailerName] !== 'function') continue
-    trailers[trailerName] = reply[kReplyTrailers][trailerName](reply, payload)
+    handled--
+
+    function cb (err, value) {
+      // TODO: we may protect multiple callback calls
+      //       or mixing async-await with callback
+      handled++
+
+      // we can safely ignore error for trailer
+      // since it does affect the client
+      // we log in here only for debug usage
+      if (err) reply.log.debug(err)
+      else trailers[trailerName] = value
+
+      // add trailers when all handler handled
+      /* istanbul ignore else */
+      if (handled === 0) {
+        res.addTrailers(trailers)
+      }
+    }
+
+    const result = reply[kReplyTrailers][trailerName](reply, payload, cb)
+    if (typeof result === 'object' && typeof result.then === 'function') {
+      result.then((v) => cb(null, v), cb)
+    } else if (result !== null && result !== undefined) {
+      // TODO: should be removed in fastify@5
+      warning.emit('FSTDEP013')
+      cb(null, result)
+    }
   }
-  res.addTrailers(trailers)
 }
 
 function sendStreamTrailer (payload, res, reply) {

lib/warnings.js

@@ -23,4 +23,6 @@ warning.create('FastifyDeprecation', 'FSTDEP011', 'Variadic listen method is dep
 
 warning.create('FastifyDeprecation', 'FSTDEP012', 'Request#context property access is deprecated. Please use "Request#routeConfig" or "Request#routeSchema" instead for accessing Route settings. The "Request#context" will be removed in `fastify@5`.')
 
+warning.create('FastifyDeprecation', 'FSTDEP013', 'Direct return of "trailers" function is deprecated. Please use "callback" or "async-await" for return value. The support of direct return will removed in `fastify@5`.')
+
 module.exports = warning

test/reply-trailers.test.js

@@ -12,8 +12,8 @@ test('send trailers when payload is empty string', t => {
   const fastify = Fastify()
 
   fastify.get('/', function (request, reply) {
-    reply.trailer('ETag', function (reply, payload) {
-      return 'custom-etag'
+    reply.trailer('ETag', function (reply, payload, done) {
+      done(null, 'custom-etag')
     })
     reply.send('')
   })
@@ -36,8 +36,8 @@ test('send trailers when payload is empty buffer', t => {
   const fastify = Fastify()
 
   fastify.get('/', function (request, reply) {
-    reply.trailer('ETag', function (reply, payload) {
-      return 'custom-etag'
+    reply.trailer('ETag', function (reply, payload, done) {
+      done(null, 'custom-etag')
     })
     reply.send(Buffer.alloc(0))
   })
@@ -60,8 +60,8 @@ test('send trailers when payload is undefined', t => {
   const fastify = Fastify()
 
   fastify.get('/', function (request, reply) {
-    reply.trailer('ETag', function (reply, payload) {
-      return 'custom-etag'
+    reply.trailer('ETag', function (reply, payload, done) {
+      done(null, 'custom-etag')
     })
     reply.send(undefined)
   })
@@ -88,11 +88,11 @@ test('send trailers when payload is json', t => {
   const md5 = hash.digest('hex')
 
   fastify.get('/', function (request, reply) {
-    reply.trailer('Content-MD5', function (reply, payload) {
+    reply.trailer('Content-MD5', function (reply, payload, done) {
       t.equal(data, payload)
       const hash = createHash('md5')
       hash.update(payload)
-      return hash.digest('hex')
+      done(null, hash.digest('hex'))
     })
     reply.send(data)
   })
@@ -116,9 +116,9 @@ test('send trailers when payload is stream', t => {
   const fastify = Fastify()
 
   fastify.get('/', function (request, reply) {
-    reply.trailer('ETag', function (reply, payload) {
+    reply.trailer('ETag', function (reply, payload, done) {
       t.same(payload, null)
-      return 'custom-etag'
+      done(null, 'custom-etag')
     })
     const stream = Readable.from([JSON.stringify({ hello: 'world' })])
     reply.send(stream)
@@ -137,19 +137,98 @@ test('send trailers when payload is stream', t => {
   })
 })
 
+test('send trailers when using async-await', t => {
+  t.plan(5)
+
+  const fastify = Fastify()
+
+  fastify.get('/', function (request, reply) {
+    reply.trailer('ETag', async function (reply, payload) {
+      return 'custom-etag'
+    })
+    reply.send('')
+  })
+
+  fastify.inject({
+    method: 'GET',
+    url: '/'
+  }, (error, res) => {
+    t.error(error)
+    t.equal(res.statusCode, 200)
+    t.equal(res.headers.trailer, 'etag')
+    t.equal(res.trailers.etag, 'custom-etag')
+    t.notHas(res.headers, 'content-length')
+  })
+})
+
+test('error in trailers should be ignored', t => {
+  t.plan(5)
+
+  const fastify = Fastify()
+
+  fastify.get('/', function (request, reply) {
+    reply.trailer('ETag', function (reply, payload, done) {
+      done('error')
+    })
+    reply.send('')
+  })
+
+  fastify.inject({
+    method: 'GET',
+    url: '/'
+  }, (error, res) => {
+    t.error(error)
+    t.equal(res.statusCode, 200)
+    t.equal(res.headers.trailer, 'etag')
+    t.notHas(res.trailers, 'etag')
+    t.notHas(res.headers, 'content-length')
+  })
+})
+
+test('should emit deprecation warning when using direct return', t => {
+  t.plan(7)
+
+  const fastify = Fastify()
+
+  fastify.get('/', function (request, reply) {
+    reply.trailer('ETag', function (reply, payload) {
+      return 'custom-etag'
+    })
+    reply.send('')
+  })
+
+  process.on('warning', onWarning)
+  function onWarning (warning) {
+    t.equal(warning.name, 'FastifyDeprecation')
+    t.equal(warning.code, 'FSTDEP013')
+  }
+  t.teardown(() => process.removeListener('warning', onWarning))
+
+  fastify.inject({
+    method: 'GET',
+    url: '/'
+  }, (error, res) => {
+    t.error(error)
+    t.equal(res.statusCode, 200)
+    t.equal(res.headers.trailer, 'etag')
+    t.equal(res.trailers.etag, 'custom-etag')
+    t.notHas(res.headers, 'content-length')
+  })
+})
+
 test('removeTrailer', t => {
   t.plan(6)
 
   const fastify = Fastify()
 
   fastify.get('/', function (request, reply) {
     reply.removeTrailer('ETag') // remove nothing
-    reply.trailer('ETag', function (reply, payload) {
-      return 'custom-etag'
+    reply.trailer('ETag', function (reply, payload, done) {
+      done(null, 'custom-etag')
     })
-    reply.trailer('Should-Not-Call', function (reply, payload) {
+    reply.trailer('Should-Not-Call', function (reply, payload, done) {
       t.fail('it should not called as this trailer is removed')
-      return 'should-not-call'
+      done(null, 'should-not-call')
     })
     reply.removeTrailer('Should-Not-Call')
     reply.send(undefined)
@@ -175,13 +254,13 @@ test('hasTrailer', t => {
 
   fastify.get('/', function (request, reply) {
     t.equal(reply.hasTrailer('ETag'), false)
-    reply.trailer('ETag', function (reply, payload) {
-      return 'custom-etag'
+    reply.trailer('ETag', function (reply, payload, done) {
+      done(null, 'custom-etag')
     })
     t.equal(reply.hasTrailer('ETag'), true)
-    reply.trailer('Should-Not-Call', function (reply, payload) {
+    reply.trailer('Should-Not-Call', function (reply, payload, done) {
       t.fail('it should not called as this trailer is removed')
-      return 'should-not-call'
+      done(null, 'should-not-call')
     })
     t.equal(reply.hasTrailer('Should-Not-Call'), true)
     reply.removeTrailer('Should-Not-Call')