Aws-xray-sdk-node: Node 10.x - captureHTTPsGlobal doesn't fully support http request interface

[mattysads]

Description

WHATWG URLs are a valid argument for http.request in Node 10 and later versions. They may be passed in place of the string argument. When enabling X-Ray, we observed a string.split error message when passing WHATWG URL parameters to http request.

This is caused by an attempt to string split the argument's path property, which does not exist on WHATWG URLs.

• https://github.com/aws/aws-xray-sdk-node/blob/master/packages/core/lib/utils.js#L40
• https://github.com/aws/aws-xray-sdk-node/blob/master/packages/core/lib/patchers/http_p.js#L102

Link reproducing the issue

To fully reproduce in a local environment, you'll need to set AWS_XRAY_CONTEXT_MISSING=LOG_ERROR, as this code path is only triggered by bypassing the missing segment error and entering this conditional.

Proposed fix

Given that the Url object isn't documented as a supported argument in Node 10 but the main branch in this codebase only officially supports Node 10, I'd propose removing support for the Url.parse object from the http patcher. This would have the potential side effect of breaking the implementation for users who are consuming this library with unsupported node versions.

If the above isn't acceptable, adding support for the WHATWG URL object in addition to the current legacy implementation is possible, but would complicate the http patcher implementation.

Node 8 and older supported an object argument in http's request method. Node 10, 12, and 14, are moving away from that functionality and are deprecating Url.parse which is consumed inside of the HTTP patcher. If it's acceptable to remove the support for Url.parse objects, I'd recommend removing the internal consumption of Url.parse at the same time and moving to an implementation that uses WHATWG URLs internally

[willarmiros]

Hi @mattysads,

Thanks for raising this, it sounds like the WHATWG URL API is available and stable in all versions of Node we support. I'd be ok with replacing our use of Url.parse with constructing a new URL object as the latest versions of Node are doing and you recommended, and replacing our references to path or any other legacy fields with their WHATWG counterparts.

If you're able to make a pull request to do this I'd greatly appreciate it, otherwise I'll add this to our backlog and make the fix as soon as I can.

[sammarks]

I too appear to have stumbled across this issue, one concrete example being with the Issuer.discover() calls inside https://github.com/panva/node-openid-client

The exact error being "Cannot read property 'split' of undefined", starting in got (where it was making the request) and the stacktrace eventually making its way back to aws-xray-sdk-core

[adabir2]

Thanks for raising this bug. We also ran into it with the got HTTP client library, which makes request using WHATWG URLs.

[hendrysadrak]

Any time soon this is released? :pray:

[willarmiros]

Released in 3.3.0