fixup

nodejs · Aug 20, 2020 · 2920be7 · 2920be7
1 parent ac32c2f
commit 2920be7
Show file tree

Hide file tree

Showing 6 changed files with 206 additions and 159 deletions.
diff --git a/lib/client.js b/lib/client.js
@@ -778,8 +778,8 @@ function connect (client) {
  socket[kPause] = socket.pause.bind(socket)
  socket[kResume] = (err) => {
  if (err) {
- // TODO: Only destroy when at head of pipeline.
- socket.destroy(err)
+ socket[kError] = err
+ resume(client)
  } else {
  socket.resume()
  }

diff --git a/lib/request.js b/lib/request.js
@@ -235,12 +235,11 @@ class Request {
  onConnect (resume) {
  this[kResume] = resume
  try {
- // TODO: Is it possible to avoid this closure?
  this[kHandler].onConnect((err) => {
- const { [kResume]: resume } = this
- if (resume) {
-  this[kResume] = null
- resume(err)
+ if (err) {
+  this.onError(err)
+ } else if (this[kResume]) {
+ this[kResume]()
  }
  })
  } catch (err) {
@@ -302,7 +301,7 @@ class Request {
  return
  }
 
- // Stop handler from touching socket after onComplete.
+ // TODO: rename aborted to finished and check that instead.
  this[kResume] = null
 
  reset.call(this)

diff --git a/test/client-abort.js b/test/client-abort.js
@@ -34,157 +34,157 @@ test('aborted response errors', (t) => {
  })
 })
 
-// test('aborted GET maxAbortedPayload reset', (t) => {
-//  t.plan(5)
-
-//  const server = createServer((req, res) => {
-//  res.end(Buffer.alloc(1e6 - 1))
-//  })
-//  t.tearDown(server.close.bind(server))
-
-//  server.listen(0, () => {
-//  const client = new Client(`http:https://localhost:${server.address().port}`, {
-//  pipelining: 2,
-//  maxAbortedPayload: 1e6
-//  })
-//  t.tearDown(client.close.bind(client))
-
-//  client.on('disconnect', () => {
-//  t.fail()
-//  })
-
-//  client.request({
-//  path: '/',
-//  method: 'GET'
-//  }, (err, { body }) => {
-//  t.error(err)
-//  body.once('data', (chunk) => {
-//  body.destroy()
-//  }).once('error', (err) => {
-//  t.ok(err)
-//  }).on('end', () => {
-//  t.fail()
-//  })
-//  })
-
-//  // Make sure read counter is reset.
-//  client.request({
-//  path: '/',
-//  method: 'GET'
-//  }, (err, { body }) => {
-//  t.error(err)
-//  body.once('data', (chunk) => {
-//  body.destroy()
-//  }).on('error', (err) => {
-//  t.ok(err)
-//  }).on('end', () => {
-//  t.fail()
-//  })
-//  })
-
-//  client.request({
-//  path: '/',
-//  method: 'GET'
-//  }, (err, { body }) => {
-//  t.error(err)
-//  body.resume()
-//  })
-//  })
-// })
-
-// test('aborted GET maxAbortedPayload', (t) => {
-//  t.plan(5)
-
-//  const server = createServer((req, res) => {
-//  res.end(Buffer.alloc(100000 + 1, 'a'))
-//  })
-//  t.tearDown(server.close.bind(server))
-
-//  server.listen(0, () => {
-//  const client = new Client(`http:https://localhost:${server.address().port}`, {
-//  pipelining: 1,
-//  maxAbortedPayload: 100000
-//  })
-//  t.tearDown(client.destroy.bind(client))
-
-//  client.request({
-//  path: '/',
-//  method: 'GET'
-//  }, (err, { body }) => {
-//  t.error(err)
-//  body.once('data', () => {
-//  body.destroy()
-//  }).once('error', (err) => {
-//  t.ok(err)
-//  })
-//  // old Readable emits error twice
-//  .on('error', () => {})
-//  })
-
-//  client.on('disconnect', () => {
-//  t.pass()
-//  })
-
-//  client.request({
-//  path: '/',
-//  method: 'GET'
-//  }, (err, { body }) => {
-//  t.error(err)
-//  body.resume()
-//  })
-
-//  client.close((err) => {
-//  t.error(err)
-//  })
-//  })
-// })
-
-// test('aborted GET maxAbortedPayload less than HWM', (t) => {
-//  t.plan(4)
-
-//  const server = createServer((req, res) => {
-//  res.end(Buffer.alloc(4 + 1, 'a'))
-//  })
-//  t.tearDown(server.close.bind(server))
-
-//  server.listen(0, () => {
-//  const client = new Client(`http:https://localhost:${server.address().port}`, {
-//  pipelining: 1,
-//  maxAbortedPayload: 4
-//  })
-//  t.tearDown(client.destroy.bind(client))
-
-//  client.request({
-//  path: '/',
-//  method: 'GET'
-//  }, (err, { body }) => {
-//  t.error(err)
-//  body.once('data', () => {
-//  body.destroy()
-//  }).once('error', (err) => {
-//  t.ok(err)
-//  })
-//  // old Readable emits error twice
-//  .on('error', () => {})
-//  })
-
-//  client.on('disconnect', () => {
-//  t.fail()
-//  })
-
-//  client.request({
-//  path: '/',
-//  method: 'GET'
-//  }, (err, { body }) => {
-//  t.error(err)
-//  body.resume()
-//  })
-
-//  client.close((err) => {
-//  t.error(err)
-//  })
-//  })
-// })
+test('aborted GET maxAbortedPayload reset', (t) => {
+ t.plan(5)
+
+ const server = createServer((req, res) => {
+ res.end(Buffer.alloc(1e6 - 1))
+ })
+ t.tearDown(server.close.bind(server))
+
+ server.listen(0, () => {
+ const client = new Client(`http:https://localhost:${server.address().port}`, {
+ pipelining: 2,
+ maxAbortedPayload: 1e6
+ })
+ t.tearDown(client.close.bind(client))
+
+ client.on('disconnect', () => {
+ t.fail()
+ })
+
+ client.request({
+ path: '/',
+ method: 'GET'
+ }, (err, { body }) => {
+ t.error(err)
+ body.once('data', (chunk) => {
+ body.destroy()
+ }).once('error', (err) => {
+ t.ok(err)
+ }).on('end', () => {
+ t.fail()
+ })
+ })
+
+ // Make sure read counter is reset.
+ client.request({
+ path: '/',
+ method: 'GET'
+ }, (err, { body }) => {
+ t.error(err)
+ body.once('data', (chunk) => {
+ body.destroy()
+ }).on('error', (err) => {
+ t.ok(err)
+ }).on('end', () => {
+ t.fail()
+ })
+ })
+
+ client.request({
+ path: '/',
+ method: 'GET'
+ }, (err, { body }) => {
+ t.error(err)
+ body.resume()
+ })
+ })
+})
+
+test('aborted GET maxAbortedPayload', (t) => {
+ t.plan(5)
+
+ const server = createServer((req, res) => {
+ res.end(Buffer.alloc(100000 + 1, 'a'))
+ })
+ t.tearDown(server.close.bind(server))
+
+ server.listen(0, () => {
+ const client = new Client(`http:https://localhost:${server.address().port}`, {
+ pipelining: 1,
+ maxAbortedPayload: 100000
+ })
+ t.tearDown(client.destroy.bind(client))
+
+ client.request({
+ path: '/',
+ method: 'GET'
+ }, (err, { body }) => {
+ t.error(err)
+ body.once('data', () => {
+ body.destroy()
+ }).once('error', (err) => {
+ t.ok(err)
+ })
+ // old Readable emits error twice
+ .on('error', () => {})
+ })
+
+ client.on('disconnect', () => {
+ t.pass()
+ })
+
+ client.request({
+ path: '/',
+ method: 'GET'
+ }, (err, { body }) => {
+ t.error(err)
+ body.resume()
+ })
+
+ client.close((err) => {
+ t.error(err)
+ })
+ })
+})
+
+test('aborted GET maxAbortedPayload less than HWM', (t) => {
+ t.plan(4)
+
+ const server = createServer((req, res) => {
+ res.end(Buffer.alloc(4 + 1, 'a'))
+ })
+ t.tearDown(server.close.bind(server))
+
+ server.listen(0, () => {
+ const client = new Client(`http:https://localhost:${server.address().port}`, {
+ pipelining: 1,
+ maxAbortedPayload: 4
+ })
+ t.tearDown(client.destroy.bind(client))
+
+ client.request({
+ path: '/',
+ method: 'GET'
+ }, (err, { body }) => {
+ t.error(err)
+ body.once('data', () => {
+ body.destroy()
+ }).once('error', (err) => {
+ t.ok(err)
+ })
+ // old Readable emits error twice
+ .on('error', () => {})
+ })
+
+ client.on('disconnect', () => {
+ t.fail()
+ })
+
+ client.request({
+ path: '/',
+ method: 'GET'
+ }, (err, { body }) => {
+ t.error(err)
+ body.resume()
+ })
+
+ client.close((err) => {
+ t.error(err)
+ })
+ })
+})
 
 test('aborted req', (t) => {
  t.plan(1)

diff --git a/test/client-dispatch.js b/test/client-dispatch.js
@@ -49,6 +49,8 @@ test('basic dispatch get', (t) => {
  method: 'GET',
  headers: reqHeaders
  }, {
+ onConnect () {
+ },
  onHeaders (statusCode, headers) {
  t.strictEqual(statusCode, 200)
  t.strictEqual(Array.isArray(headers), true)
@@ -99,6 +101,8 @@ test('trailers dispatch get', (t) => {
  method: 'GET',
  headers: reqHeaders
  }, {
+ onConnect () {
+ },
  onHeaders (statusCode, headers) {
  t.strictEqual(statusCode, 200)
  t.strictEqual(Array.isArray(headers), true)
@@ -142,6 +146,8 @@ test('dispatch onHeaders error', (t) => {
  path: '/',
  method: 'GET'
  }, {
+ onConnect () {
+ },
  onHeaders (statusCode, headers) {
  throw _err
  },
@@ -175,6 +181,8 @@ test('dispatch onComplete error', (t) => {
  path: '/',
  method: 'GET'
  }, {
+ onConnect () {
+ },
  onHeaders (statusCode, headers) {
  t.pass()
  },
@@ -208,6 +216,8 @@ test('dispatch onData error', (t) => {
  path: '/',
  method: 'GET'
  }, {
+ onConnect () {
+ },
  onHeaders (statusCode, headers) {
  t.pass()
  },