Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(interceptors): migrate decorator handler to new hooks #3905

Merged
merged 6 commits into from
Dec 1, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 23 additions & 27 deletions lib/handler/decorator-handler.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict'

const assert = require('node:assert')
const WrapHandler = require('./wrap-handler')

/**
* @deprecated
Expand All @@ -9,63 +10,58 @@ module.exports = class DecoratorHandler {
#handler
#onCompleteCalled = false
#onErrorCalled = false
#onResponseStartCalled = false

constructor (handler) {
if (typeof handler !== 'object' || handler === null) {
throw new TypeError('handler must be an object')
}
this.#handler = handler
this.#handler = WrapHandler.wrap(handler)
}

onConnect (...args) {
return this.#handler.onConnect?.(...args)
onRequestStart (...args) {
return this.#handler.onRequestStart?.(...args)
}

onError (...args) {
this.#onErrorCalled = true
return this.#handler.onError?.(...args)
}

onUpgrade (...args) {
onRequestUpgrade (...args) {
assert(!this.#onCompleteCalled)
assert(!this.#onErrorCalled)

return this.#handler.onUpgrade?.(...args)
return this.#handler.onRequestUpgrade?.(...args)
}

onResponseStarted (...args) {
onResponseStart (...args) {
assert(!this.#onCompleteCalled)
assert(!this.#onErrorCalled)
assert(!this.#onResponseStartCalled)

return this.#handler.onResponseStarted?.(...args)
}

onHeaders (...args) {
assert(!this.#onCompleteCalled)
assert(!this.#onErrorCalled)
this.#onResponseStartCalled = true

return this.#handler.onHeaders?.(...args)
return this.#handler.onResponseStart?.(...args)
}

onData (...args) {
onResponseData (...args) {
assert(!this.#onCompleteCalled)
assert(!this.#onErrorCalled)

return this.#handler.onData?.(...args)
return this.#handler.onResponseData?.(...args)
}

onComplete (...args) {
onResponseEnd (...args) {
assert(!this.#onCompleteCalled)
assert(!this.#onErrorCalled)

this.#onCompleteCalled = true
return this.#handler.onComplete?.(...args)
return this.#handler.onResponseEnd?.(...args)
}

onBodySent (...args) {
assert(!this.#onCompleteCalled)
assert(!this.#onErrorCalled)

return this.#handler.onBodySent?.(...args)
onResponseError (...args) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe we should introduce a new hook to handle pre-request or in-flight request errors (e.g. network timeouts after connect, or runtime errors before connecting with remote peer); it feels quite odd handling these kind of errors through onResponseError when the response has not even being received

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

onRequestError?

this.#onErrorCalled = true
return this.#handler.onResponseError?.(...args)
}

/**
* @deprecated
*/
onBodySent () {}
}
28 changes: 3 additions & 25 deletions lib/interceptor/dns.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
const { isIP } = require('node:net')
const { lookup } = require('node:dns')
const DecoratorHandler = require('../handler/decorator-handler')
const WrapHandler = require('../handler/wrap-handler')

Check failure on line 5 in lib/interceptor/dns.js

View workflow job for this annotation

GitHub Actions / Lint

'WrapHandler' is assigned a value but never used
const { InvalidArgumentError, InformationalError } = require('../core/errors')
const maxInt = Math.pow(2, 31) - 1

Expand Down Expand Up @@ -223,39 +223,17 @@
#state = null
#opts = null
#dispatch = null
#handler = null
#origin = null
#controller = null

constructor (state, { origin, handler, dispatch }, opts) {
super(handler)
this.#origin = origin
this.#handler = WrapHandler.wrap(handler)
this.#opts = { ...opts }
this.#state = state
this.#dispatch = dispatch
}

onRequestStart (controller, context) {
this.#handler.onRequestStart?.(controller, context)
}

onRequestUpgrade (controller, statusCode, headers, socket) {
this.handler.onRequestUpgrade?.(controller, statusCode, headers, socket)
}

onResponseStart (controller, statusCode, headers, statusMessage) {
this.#handler.onResponseStart?.(controller, statusCode, headers, statusMessage)
}

onResponseData (controller, data) {
this.#handler.onResponseData?.(controller, data)
}

onResponseEnd (controller, trailers) {
this.#handler.onResponseEnd?.(controller, trailers)
}

onResponseError (controller, err) {
switch (err.code) {
case 'ETIMEDOUT':
Expand All @@ -264,7 +242,7 @@
// We delete the record and retry
this.#state.runLookup(this.#origin, this.#opts, (err, newOrigin) => {
if (err) {
this.#handler.onResponseError(controller, err)
super.onResponseError(controller, err)
return
}

Expand All @@ -280,14 +258,14 @@
}

// if dual-stack disabled, we error out
this.#handler.onResponseError(controller, err)
super.onResponseError(controller, err)
break
}
case 'ENOTFOUND':
this.#state.deleteRecord(this.#origin)
// eslint-disable-next-line no-fallthrough
default:
this.#handler.onResponseError(controller, err)
super.onResponseError(controller, err)
break
}
}
Expand Down
70 changes: 29 additions & 41 deletions lib/interceptor/dump.js
Original file line number Diff line number Diff line change
@@ -1,43 +1,40 @@
'use strict'

const util = require('../core/util')
const { InvalidArgumentError, RequestAbortedError } = require('../core/errors')
const DecoratorHandler = require('../handler/decorator-handler')

class DumpHandler extends DecoratorHandler {
#maxSize = 1024 * 1024
#abort = null
#dumped = false
#aborted = false
#size = 0
#reason = null
#handler = null
#controller = null
aborted = false
reason = false

constructor ({ maxSize }, handler) {
constructor ({ maxSize, signal }, handler) {
if (maxSize != null && (!Number.isFinite(maxSize) || maxSize < 1)) {
throw new InvalidArgumentError('maxSize must be a number greater than 0')
}

super(handler)

this.#maxSize = maxSize ?? this.#maxSize
this.#handler = handler
// this.#handler = handler
}

onConnect (abort) {
this.#abort = abort

this.#handler.onConnect(this.#customAbort.bind(this))
#abort (reason) {
this.aborted = true
this.reason = reason
}

#customAbort (reason) {
this.#aborted = true
this.#reason = reason
onRequestStart (controller, context) {
controller.abort = this.#abort.bind(this)
this.#controller = controller

return super.onRequestStart(controller, context)
}

// TODO: will require adjustment after new hooks are out
onHeaders (statusCode, rawHeaders, resume, statusMessage) {
const headers = util.parseHeaders(rawHeaders)
onResponseStart (controller, statusCode, headers, statusMessage) {
const contentLength = headers['content-length']

if (contentLength != null && contentLength > this.#maxSize) {
Expand All @@ -48,55 +45,50 @@ class DumpHandler extends DecoratorHandler {
)
}

if (this.#aborted) {
if (this.aborted === true) {
return true
}

return this.#handler.onHeaders(
statusCode,
rawHeaders,
resume,
statusMessage
)
return super.onResponseStart(controller, statusCode, headers, statusMessage)
}

onError (err) {
onResponseError (controller, err) {
if (this.#dumped) {
return
}

err = this.#reason ?? err
err = this.#controller.reason ?? err

this.#handler.onError(err)
super.onResponseError(controller, err)
}

onData (chunk) {
onResponseData (controller, chunk) {
this.#size = this.#size + chunk.length

if (this.#size >= this.#maxSize) {
this.#dumped = true

if (this.#aborted) {
this.#handler.onError(this.#reason)
if (this.aborted === true) {
super.onResponseError(controller, this.reason)
} else {
this.#handler.onComplete([])
super.onResponseEnd(controller, {})
}
}

return true
}

onComplete (trailers) {
onResponseEnd (controller, trailers) {
if (this.#dumped) {
return
}

if (this.#aborted) {
this.#handler.onError(this.reason)
if (this.#controller.aborted === true) {
super.onResponseError(controller, this.reason)
return
}

this.#handler.onComplete(trailers)
super.onResponseEnd(controller, trailers)
}
}

Expand All @@ -107,13 +99,9 @@ function createDumpInterceptor (
) {
return dispatch => {
return function Intercept (opts, handler) {
const { dumpMaxSize = defaultMaxSize } =
opts
const { dumpMaxSize = defaultMaxSize } = opts

const dumpHandler = new DumpHandler(
{ maxSize: dumpMaxSize },
handler
)
const dumpHandler = new DumpHandler({ maxSize: dumpMaxSize, signal: opts.signal }, handler)

return dispatch(opts, dumpHandler)
}
Expand Down
36 changes: 17 additions & 19 deletions lib/interceptor/response-error.js
Original file line number Diff line number Diff line change
@@ -1,59 +1,57 @@
'use strict'

const { parseHeaders } = require('../core/util')
// const { parseHeaders } = require('../core/util')
const DecoratorHandler = require('../handler/decorator-handler')
const { ResponseError } = require('../core/errors')

class ResponseErrorHandler extends DecoratorHandler {
#handler
#statusCode
#contentType
#decoder
#headers
#body

constructor (opts, { handler }) {
constructor (_opts, { handler }) {
super(handler)
this.#handler = handler
}

onConnect (abort) {
#checkContentType (contentType) {
return this.#contentType.indexOf(contentType) === 0
}

onRequestStart (controller, context) {
this.#statusCode = 0
this.#contentType = null
this.#decoder = null
this.#headers = null
this.#body = ''

return this.#handler.onConnect(abort)
}

#checkContentType (contentType) {
return this.#contentType.indexOf(contentType) === 0
return super.onRequestStart(controller, context)
}

onHeaders (statusCode, rawHeaders, resume, statusMessage, headers = parseHeaders(rawHeaders)) {
onResponseStart (controller, statusCode, headers, statusMessage) {
this.#statusCode = statusCode
this.#headers = headers
this.#contentType = headers['content-type']

if (this.#statusCode < 400) {
return this.#handler.onHeaders(statusCode, rawHeaders, resume, statusMessage, headers)
return super.onResponseStart(controller, statusCode, headers, statusMessage)
}

if (this.#checkContentType('application/json') || this.#checkContentType('text/plain')) {
this.#decoder = new TextDecoder('utf-8')
}
}

onData (chunk) {
onResponseData (controller, chunk) {
if (this.#statusCode < 400) {
return this.#handler.onData(chunk)
return super.onResponseData(controller, chunk)
}

this.#body += this.#decoder?.decode(chunk, { stream: true }) ?? ''
}

onComplete (rawTrailers) {
onResponseEnd (controller, trailers) {
if (this.#statusCode >= 400) {
this.#body += this.#decoder?.decode(undefined, { stream: false }) ?? ''

Expand All @@ -77,14 +75,14 @@ class ResponseErrorHandler extends DecoratorHandler {
Error.stackTraceLimit = stackTraceLimit
}

this.#handler.onError(err)
super.onResponseError(controller, err)
} else {
this.#handler.onComplete(rawTrailers)
super.onResponseEnd(controller, trailers)
}
}

onError (err) {
this.#handler.onError(err)
onResponseError (err) {
super.onResponseError(err)
}
}

Expand Down
Loading