patch-http not supporting "new" node URL object, breaking all outgoing calls

[adabir2]

Summary

Deviating from the template to give you a summary that'll hopefully quickly communicate the problem:
I think the node HTTP module patching in this project isn't handling a common call pattern. Looking at the code, the patch-http file seems to only support what the node docs call the legacy URL object, which is now deprecated. Things fail if an http.request() call is made using the new node URL object.

Expected Behavior

Patching the node HTTP module doesn't interfere with the rest of the application. HTTP requests work as expected.

Actual Behavior

All HTTP requests are being sent to localhost because the URL portion isn't interpreted correctly.

Steps to Reproduce the Problem

I think this test case should help you reproduce it if you add it to your src/trace/patch-http.spec.ts file:

  it("injects tracing headers when using the new WHATWG URL object", () => {
    nock("http://www.example.com").get("/").reply(200, {});
    patchHttp(contextService);
    // const url = parse("http://www.example.com");
    const url = new URL("http://www.example.com");
    const req = http.request(url);
    expectHeaders(req);
  });

Specifications

• Datadog Lambda Layer version: My app failed with v3.29.0, but I did try the unit test above with your latest master v3.41.0.
• Node version: 14.3.0

Stacktrace

Not relevant

Detailed description of the problem

I have a lambda that is wrapped with datadog-lambda-js. The lambda's business logic needs to send out some HTTP requests. I'm using the GOT library for that: got

With the wrapping, all http requests are being sent to 127.0.0.1:443! Things work correctly if I remove datadog-lambda-js.

After much code reading, I think I've identified the problem. Your code from patch-http.ts:

function normalizeArgs(
  arg1: string | URL | http.RequestOptions,
  arg2?: RequestCallback | http.RequestOptions,
  arg3?: RequestCallback,
) {
  let options: http.RequestOptions = typeof arg1 === "string" ? parse(arg1) : { ...arg1 };

The got library is calling http.request with 2 args where arg1 is an instance of the "new" URL object in node. The object is not a simple JS object that you can use a spread operator on, as is being done in the ternary statement above.

The code above looks like it only works with the now deprecated legacy URL object. The legacy one was returning simple objects.

[DarcyRaynerDD]

Hi @adabir2 . Thanks for the detailed report, this looks like a fairly serious issue. We will try and fix this soon.

[adabir2]

Great, thank you. I sympathize with you having to deal with so many different ways of calling http.request!

[DarcyRaynerDD]

Hi @adabir2 . Sorry about the delay getting back to you on this. v43 of the layer contains a fix for this issue #146 . I've tested this myself and believe it addresses all cases now. Let use know if you run in to any issues.

[adabir2]

Excellent. Thanks a lot @DarcyRaynerDD