Skip to content

Commit

Permalink
Revert "fix: don't leak internal class (#3024)"
Browse files Browse the repository at this point in the history
This reverts commit 2d5cbdf.
  • Loading branch information
mcollina committed Apr 2, 2024
1 parent d542b8c commit 1a0c73d
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 36 deletions.
2 changes: 2 additions & 0 deletions docs/docs/api/DiagnosticsChannel.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ diagnosticsChannel.channel('undici:request:create').subscribe(({ request }) => {
console.log('method', request.method)
console.log('path', request.path)
console.log('headers') // array of strings, e.g: ['foo', 'bar']
request.addHeader('hello', 'world')
console.log('headers', request.headers) // e.g. ['foo', 'bar', 'hello', 'world']
})
```

Expand Down
39 changes: 10 additions & 29 deletions lib/core/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,6 @@ class Request {

this.abort = null

this.publicInterface = null

if (body == null) {
this.body = null
} else if (isStream(body)) {
Expand Down Expand Up @@ -189,32 +187,10 @@ class Request {
this[kHandler] = handler

if (channels.create.hasSubscribers) {
channels.create.publish({ request: this.getPublicInterface() })
channels.create.publish({ request: this })
}
}

getPublicInterface () {
const self = this
this.publicInterface ??= {
get origin () {
return self.origin
},
get method () {
return self.method
},
get path () {
return self.path
},
get headers () {
return self.headers
},
get completed () {
return self.completed
}
}
return this.publicInterface
}

onBodySent (chunk) {
if (this[kHandler].onBodySent) {
try {
Expand All @@ -227,7 +203,7 @@ class Request {

onRequestSent () {
if (channels.bodySent.hasSubscribers) {
channels.bodySent.publish({ request: this.getPublicInterface() })
channels.bodySent.publish({ request: this })
}

if (this[kHandler].onRequestSent) {
Expand Down Expand Up @@ -260,7 +236,7 @@ class Request {
assert(!this.completed)

if (channels.headers.hasSubscribers) {
channels.headers.publish({ request: this.getPublicInterface(), response: { statusCode, headers, statusText } })
channels.headers.publish({ request: this, response: { statusCode, headers, statusText } })
}

try {
Expand Down Expand Up @@ -296,7 +272,7 @@ class Request {

this.completed = true
if (channels.trailers.hasSubscribers) {
channels.trailers.publish({ request: this.getPublicInterface(), trailers })
channels.trailers.publish({ request: this, trailers })
}

try {
Expand All @@ -311,7 +287,7 @@ class Request {
this.onFinally()

if (channels.error.hasSubscribers) {
channels.error.publish({ request: this.getPublicInterface(), error })
channels.error.publish({ request: this, error })
}

if (this.aborted) {
Expand All @@ -333,6 +309,11 @@ class Request {
this.endHandler = null
}
}

addHeader (key, value) {
processHeader(this, key, value)
return this
}
}

function processHeader (request, key, val) {
Expand Down
2 changes: 1 addition & 1 deletion lib/dispatcher/client-h1.js
Original file line number Diff line number Diff line change
Expand Up @@ -993,7 +993,7 @@ function writeH1 (client, request) {
}

if (channels.sendHeaders.hasSubscribers) {
channels.sendHeaders.publish({ request: request.getPublicInterface(), headers: header, socket })
channels.sendHeaders.publish({ request, headers: header, socket })
}

/* istanbul ignore else: assertion */
Expand Down
7 changes: 5 additions & 2 deletions test/node-test/diagnostics-channel/get.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const { Client } = require('../../..')
const { createServer } = require('node:http')

test('Diagnostics channel - get', (t) => {
const assert = tspl(t, { plan: 31 })
const assert = tspl(t, { plan: 32 })
const server = createServer((req, res) => {
res.setHeader('Content-Type', 'text/plain')
res.setHeader('trailer', 'foo')
Expand All @@ -33,6 +33,8 @@ test('Diagnostics channel - get', (t) => {
assert.equal(request.method, 'GET')
assert.equal(request.path, '/')
assert.deepStrictEqual(request.headers, ['bar', 'bar'])
request.addHeader('hello', 'world')
assert.deepStrictEqual(request.headers, ['bar', 'bar', 'hello', 'world'])
})

let _connector
Expand Down Expand Up @@ -75,7 +77,8 @@ test('Diagnostics channel - get', (t) => {
'GET / HTTP/1.1',
`host: localhost:${server.address().port}`,
'connection: keep-alive',
'bar: bar'
'bar: bar',
'hello: world'
]

assert.deepStrictEqual(headers, expectedHeaders.join('\r\n') + '\r\n')
Expand Down
8 changes: 6 additions & 2 deletions test/node-test/diagnostics-channel/post-stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ const { Client } = require('../../..')
const { createServer } = require('node:http')

test('Diagnostics channel - post stream', (t) => {
const assert = tspl(t, { plan: 31 })
const assert = tspl(t, { plan: 33 })
const server = createServer((req, res) => {
req.resume()
res.setHeader('Content-Type', 'text/plain')
Expand All @@ -34,6 +34,9 @@ test('Diagnostics channel - post stream', (t) => {
assert.equal(request.method, 'POST')
assert.equal(request.path, '/')
assert.deepStrictEqual(request.headers, ['bar', 'bar'])
request.addHeader('hello', 'world')
assert.deepStrictEqual(request.headers, ['bar', 'bar', 'hello', 'world'])
assert.deepStrictEqual(request.body, body)
})

let _connector
Expand Down Expand Up @@ -76,7 +79,8 @@ test('Diagnostics channel - post stream', (t) => {
'POST / HTTP/1.1',
`host: localhost:${server.address().port}`,
'connection: keep-alive',
'bar: bar'
'bar: bar',
'hello: world'
]

assert.equal(headers, expectedHeaders.join('\r\n') + '\r\n')
Expand Down
8 changes: 6 additions & 2 deletions test/node-test/diagnostics-channel/post.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const { Client } = require('../../../')
const { createServer } = require('node:http')

test('Diagnostics channel - post', (t) => {
const assert = tspl(t, { plan: 31 })
const assert = tspl(t, { plan: 33 })
const server = createServer((req, res) => {
req.resume()
res.setHeader('Content-Type', 'text/plain')
Expand All @@ -32,6 +32,9 @@ test('Diagnostics channel - post', (t) => {
assert.equal(request.method, 'POST')
assert.equal(request.path, '/')
assert.deepStrictEqual(request.headers, ['bar', 'bar'])
request.addHeader('hello', 'world')
assert.deepStrictEqual(request.headers, ['bar', 'bar', 'hello', 'world'])
assert.deepStrictEqual(request.body, Buffer.from('hello world'))
})

let _connector
Expand Down Expand Up @@ -74,7 +77,8 @@ test('Diagnostics channel - post', (t) => {
'POST / HTTP/1.1',
`host: localhost:${server.address().port}`,
'connection: keep-alive',
'bar: bar'
'bar: bar',
'hello: world'
]

assert.deepStrictEqual(headers, expectedHeaders.join('\r\n') + '\r\n')
Expand Down

0 comments on commit 1a0c73d

Please sign in to comment.