Skip to content

Commit

Permalink
cleanup
Browse files Browse the repository at this point in the history
Signed-off-by: Matteo Collina <[email protected]>
  • Loading branch information
mcollina committed Nov 14, 2024
1 parent b471d20 commit 7b54feb
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 62 deletions.
16 changes: 1 addition & 15 deletions lib/core/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,7 @@ const { headerNameLowerCasedRecord } = require('./constants')
const invalidPathRegex = /[^\u0021-\u00ff]/

const kHandler = Symbol('handler')
const kCompleted = Symbol('completed')

let nextRequestId = 0
class Request {
constructor (origin, {
path,
Expand All @@ -46,7 +44,6 @@ class Request {
servername,
throwOnError
}, handler) {
this.id = nextRequestId++
if (typeof path !== 'string') {
throw new InvalidArgumentError('path must be a string')
} else if (
Expand Down Expand Up @@ -132,18 +129,7 @@ class Request {
throw new InvalidArgumentError('body must be a string, a Buffer, a Readable stream, an iterable, or an async iterable')
}

this[kCompleted] = false

Object.defineProperty(this, 'completed', {
get () {
return this[kCompleted]
},
set (value) {
process._rawDebug(this.id, 'completed set to', value, 'from', this[kCompleted], Error().stack)
this[kCompleted] = value
}
})

this.completed = false
this.aborted = false

this.upgrade = upgrade || null
Expand Down
45 changes: 2 additions & 43 deletions lib/dispatcher/client-h2.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,6 @@ function parseH2Headers (headers) {
return result
}

let sessionId = 0

async function connectH2 (client, socket) {
client[kSocket] = socket

Expand All @@ -93,8 +91,6 @@ async function connectH2 (client, socket) {
peerMaxConcurrentStreams: client[kMaxConcurrentStreams]
})

session.id = sessionId++

session[kOpenStreams] = 0
session[kClient] = client
session[kSocket] = socket
Expand Down Expand Up @@ -198,8 +194,6 @@ function onHttp2SessionGoAway (errorCode) {
client[kSocket] = null
client[kHTTPContext] = null

process._rawDebug('goaway', this)

// this is an HTTP2 session
this.close()
this[kHTTP2Session] = null
Expand Down Expand Up @@ -283,14 +277,7 @@ function shouldSendContentLength (method) {
return method !== 'GET' && method !== 'HEAD' && method !== 'OPTIONS' && method !== 'TRACE' && method !== 'CONNECT'
}

const seen = new Set()

function writeH2 (client, request) {
assert(!request.completed)
if (seen.has(request.id)) {
throw new Error('This request was already seen ' + request.id)
}
seen.add(request.id)
const session = client[kHTTP2Session]
const { body, method, path, host, upgrade, expectContinue, signal, headers: reqHeaders } = request

Expand Down Expand Up @@ -342,9 +329,6 @@ function writeH2 (client, request) {
// On Abort, we close the stream to send RST_STREAM frame
stream.close()

console.log('kRunningIdx', client[kRunningIdx])
console.log('kQueue', client[kQueue])

// We move the running index to the next request
client[kOnError](err)
client[kResume]()
Expand Down Expand Up @@ -472,10 +456,6 @@ function writeH2 (client, request) {
// Increment counter as we have new streams open
++session[kOpenStreams]

// Unfortunately, there is a bug in HTTP/2 that have 'data' being
// emitted after 'end'
let endEmitted = false

stream.once('response', headers => {
const { [HTTP2_HEADER_STATUS]: statusCode, ...realHeaders } = headers
request.onResponseStarted()
Expand All @@ -496,34 +476,17 @@ function writeH2 (client, request) {
})

stream.on('data', (chunk) => {
try {
process._rawDebug(session.id, stream.id, request.id, 'completed', request.completed)
if (request.completed) {
process._rawDebug('---')
process._rawDebug(session.id, stream.id, request.id, endEmitted)
process._rawDebug(stream)
process._rawDebug(session)
process._rawDebug(request)
process._rawDebug('---')
}
if (request.onData(chunk) === false) {
stream.pause()
}
} catch (err) {
process._rawDebug('caught', err)
stream.destroy(err)
if (request.onData(chunk) === false) {
stream.pause()
}
})

stream.once('end', (err) => {
process._rawDebug(session.id, stream.id, request.id, 'end emitted')
endEmitted = true
stream.removeAllListeners('data')
// When state is null, it means we haven't consumed body and the stream still do not have
// a state.
// Present specially when using pipeline or stream
if (stream.state?.state == null || stream.state.state < 6) {
process._rawDebug(session.id, stream.id, request.id, 'calling on complete')
// The request was aborted
if (!request.aborted) {
request.onComplete([])
Expand All @@ -532,7 +495,6 @@ function writeH2 (client, request) {
client[kQueue][client[kRunningIdx]++] = null
client[kResume]()
} else {
process._rawDebug(session.id, stream.id, request.id, 'not calling on complete')
// Stream is closed or half-closed-remote (6), decrement counter and cleanup
// It does not have sense to continue working with the stream as we do not
// have yet RST_STREAM support on client-side
Expand All @@ -557,7 +519,6 @@ function writeH2 (client, request) {
})

stream.once('error', function (err) {
process._rawDebug(session.id, stream.id, request.id, 'error', err)
stream.removeAllListeners('data')
abort(err)
})
Expand All @@ -568,9 +529,7 @@ function writeH2 (client, request) {
})

stream.on('aborted', () => {
process._rawDebug(session.id, stream.id, request.id, 'aborted')
stream.removeAllListeners('data')
endEmitted = true
})

// stream.on('timeout', () => {
Expand Down
4 changes: 0 additions & 4 deletions test/http2.js
Original file line number Diff line number Diff line change
Expand Up @@ -1400,7 +1400,6 @@ test('#2364 - Concurrent aborts (2nd variant)', async t => {
}
},
(err, response) => {
process._rawDebug('response 1')
t.ifError(err)
t.strictEqual(
response.headers['content-type'],
Expand All @@ -1421,7 +1420,6 @@ test('#2364 - Concurrent aborts (2nd variant)', async t => {
signal
},
(err, response) => {
process._rawDebug('response 2')
t.strictEqual(err.name, 'TimeoutError')
}
)
Expand All @@ -1435,7 +1433,6 @@ test('#2364 - Concurrent aborts (2nd variant)', async t => {
}
},
(err, response) => {
process._rawDebug('response 3')
t.ifError(err)
t.strictEqual(
response.headers['content-type'],
Expand All @@ -1456,7 +1453,6 @@ test('#2364 - Concurrent aborts (2nd variant)', async t => {
signal
},
(err, response) => {
process._rawDebug('response 4')
t.strictEqual(err.name, 'TimeoutError')
}
)
Expand Down

0 comments on commit 7b54feb

Please sign in to comment.