Skip to content

Commit

Permalink
Revert "fixup! Simplify reply sent monitoring (#3072) (#3079)"
Browse files Browse the repository at this point in the history
This reverts commit 9bf6a33.
  • Loading branch information
mcollina committed May 29, 2021
1 parent 4346d9a commit 5deae9d
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 11 deletions.
8 changes: 6 additions & 2 deletions lib/handleRequest.js
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,9 @@ function handler (request, reply) {
}

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

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

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

if (err != null) {
reply.send(err)
Expand Down
6 changes: 2 additions & 4 deletions lib/reply.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,9 @@ 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. The response.finished will be always true when the request
// method is HEAD and http2 is used, so we have to combine that with
// response.headersSent.
// applicable.
// 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.headersSent && this.raw.finished))) === true
return (this[kReplySentOverwritten] || (typeof this.raw.writableEnded !== 'undefined' ? this.raw.writableEnded : this.raw.finished)) === true
},
set (value) {
if (value !== true) {
Expand Down
11 changes: 8 additions & 3 deletions test/internals/handleRequest.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'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 @@ -107,8 +108,12 @@ test('handler function - preValidationCallback with finished response', t => {
t.plan(0)
const res = {}
// Be sure to check only `writableEnded` where is available
res.writableEnded = true

if (semver.gte(process.versions.node, '12.9.0')) {
res.writableEnded = true
} else {
res.writable = false
res.finished = true
}
res.end = () => {
t.fail()
}
Expand All @@ -133,7 +138,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.headersSent = true
res.writable = false
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.headersSent and response.finished if response.writableEnded is not defined', t => {
test('reply.sent should read from response.finished if response.writableEnded is not defined', t => {
t.plan(1)

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

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

0 comments on commit 5deae9d

Please sign in to comment.