diff --git a/docs/Reference/Reply.md b/docs/Reference/Reply.md index c669790927..b5d690ae83 100644 --- a/docs/Reference/Reply.md +++ b/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') diff --git a/lib/reply.js b/lib/reply.js index 68f02de06d..04fbd0fe8a 100644 --- a/lib/reply.js +++ b/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) { diff --git a/lib/warnings.js b/lib/warnings.js index 76d15e1513..25fe27be8a 100644 --- a/lib/warnings.js +++ b/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 diff --git a/test/reply-trailers.test.js b/test/reply-trailers.test.js index e9ae60cdf7..ee59ed3353 100644 --- a/test/reply-trailers.test.js +++ b/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,6 +137,85 @@ 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) @@ -144,12 +223,12 @@ test('removeTrailer', t => { 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')