Skip to content

Commit

Permalink
fixup! Simplify reply sent monitoring (fastify#3072)
Browse files Browse the repository at this point in the history
  • Loading branch information
sergejostir committed May 17, 2021
1 parent a5386e4 commit 75c6bb9
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 18 deletions.
8 changes: 2 additions & 6 deletions lib/handleRequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,7 @@ function handler (request, reply) {
}

function preValidationCallback (err, request, reply) {
if (reply.sent === true ||
reply.raw.writableEnded === true ||
reply.raw.writable === false) return
if (reply.sent === true) return

if (err != null) {
reply.send(err)
Expand Down Expand Up @@ -109,9 +107,7 @@ function preValidationCallback (err, request, reply) {
}

function preHandlerCallback (err, request, reply) {
if (reply.sent ||
reply.raw.writableEnded === true ||
reply.raw.writable === false) return
if (reply.sent) return

if (err != null) {
reply.send(err)
Expand Down
5 changes: 3 additions & 2 deletions lib/reply.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,10 @@ Object.defineProperties(Reply.prototype, {
// response has ended. The response.writableEnded property was added in
// Node.js v12.9.0. Since fastify supports older Node.js versions as well,
// we have to take response.finished property in consideration when
// applicable.
// applicable. The response.finished will always be true when the request
// method is HEAD, so we have to combine that with response.headersSent.
// TODO: remove fallback when the lowest supported Node.js version >= v12.9.0
return (this[kReplySentOverwritten] || (typeof this.raw.writableEnded !== 'undefined' ? this.raw.writableEnded : this.raw.finished)) === true
return (this[kReplySentOverwritten] || (typeof this.raw.writableEnded !== 'undefined' ? this.raw.writableEnded : (this.raw.headersSent && this.raw.finished))) === true
},
set (value) {
if (value !== true) {
Expand Down
11 changes: 3 additions & 8 deletions test/internals/handleRequest.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
'use strict'

const { test } = require('tap')
const semver = require('semver')
const handleRequest = require('../../lib/handleRequest')
const internals = require('../../lib/handleRequest')[Symbol.for('internals')]
const Request = require('../../lib/request')
Expand Down Expand Up @@ -108,12 +107,8 @@ test('handler function - preValidationCallback with finished response', t => {
t.plan(0)
const res = {}
// Be sure to check only `writableEnded` where is available
if (semver.gte(process.versions.node, '12.9.0')) {
res.writableEnded = true
} else {
res.writable = false
res.finished = true
}
res.writableEnded = true

res.end = () => {
t.fail()
}
Expand All @@ -138,7 +133,7 @@ test('handler function - preValidationCallback with finished response (< v12.9.0
t.plan(0)
const res = {}
// Be sure to check only `writableEnded` where is available
res.writable = false
res.headersSent = true
res.finished = true

res.end = () => {
Expand Down
4 changes: 2 additions & 2 deletions test/internals/reply.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1694,10 +1694,10 @@ test('reply.sent should read from response.writableEnded if it is defined', t =>
t.equal(reply.sent, true)
})

test('reply.sent should read from response.finished if response.writableEnded is not defined', t => {
test('reply.sent should read from response.headersSent and response.finished if response.writableEnded is not defined', t => {
t.plan(1)

const reply = new Reply({ finished: true }, {}, {})
const reply = new Reply({ headersSent: true, finished: true }, {}, {})

t.equal(reply.sent, true)
})

0 comments on commit 75c6bb9

Please sign in to comment.