Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: support async trailer #4380

Merged
merged 6 commits into from Oct 27, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 11 additions & 3 deletions docs/Reference/Reply.md
Expand Up @@ -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() {
Expand All @@ -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')
Expand Down
31 changes: 29 additions & 2 deletions lib/reply.js
Expand Up @@ -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) {
Expand Down
2 changes: 2 additions & 0 deletions lib/warnings.js
Expand Up @@ -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
115 changes: 97 additions & 18 deletions test/reply-trailers.test.js
Expand Up @@ -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('')
})
Expand All @@ -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))
})
Expand All @@ -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)
})
Expand All @@ -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)
})
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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')
Expand Down