fix: user-agent by making __filename check more resilient

[Ethan-Arrowood]

Rel: #2310 nodejs/node#50145

For some reason __filename is undefined when this gets loaded into node.js

Trying something else that might be more resilient. This also just falls back to index.js so that if all goes wrong at least we don't get a runtime error. WDYT? Should we switch the default to result in 'node'?

[codecov-commenter]

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (e39a632) 85.54% compared to head (7377b9b) 92.28%.
Report is 42 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2339      +/-   ##
==========================================
+ Coverage   85.54%   92.28%   +6.73%     
==========================================
  Files          76       41      -35     
  Lines        6858     3253    -3605     
==========================================
- Hits         5867     3002    -2865     
+ Misses        991      251     -740     

Files	Coverage Δ
lib/core/connect.js	80.00% <100.00%> (-1.25%)	⬇️
lib/core/request.js	88.09% <100.00%> (+1.01%)	⬆️
lib/pool.js	100.00% <100.00%> (ø)
lib/client.js	93.18% <92.30%> (+0.05%)	⬆️
lib/core/util.js	82.63% <85.71%> (-13.16%)	⬇️

... and 39 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Ethan-Arrowood]

@KhafraDev previously commented that there isn't a difference between __filename and globalThis.__filename.

Unless someone else has a better idea, I think we rely on this undefined behavior to set the agent to 'node'. We can do:

(__filename ?? '').endsWith('index.js') ? 'undici' : 'node'

or

__filename ? 'undici' : 'node'

I prefer the first so that if node ever enables __filename in its deps, then the code won't magically switch to 'undici' user-agent.

WDYT?

[KhafraDev]

this would still throw an error if __filename isn't defined, so it could be simplified to globalThis.__filename ?? 'index.js' or we could use #2340

[Ethan-Arrowood]

I prefer this with the globalThis.__filename change. that way no random change to globals could break it.

[KhafraDev]

or maybe just typeof __filename !== 'undefined' ? 'undici' : 'node'

[Ethan-Arrowood]

If Node.js changes its behavior that would break undici by reverting it back to 'undici'

[KhafraDev]

a few tests are failing, the main entry point for undici is index.js which probably has something to do with it

[Ethan-Arrowood]

lol globalThis doesn't have __filename on it

TAP version 13
globalThis <ref *1> Object [global] {
  global: [Circular *1],
  clearInterval: [Function: clearInterval],
  clearTimeout: [Function: clearTimeout],
  setInterval: [Function: setInterval],
  setTimeout: [Function: setTimeout] {
    [Symbol(nodejs.util.promisify.custom)]: [Getter]
  },
  queueMicrotask: [Function: queueMicrotask],
  performance: Performance {
    nodeTiming: PerformanceNodeTiming {
      name: 'node',
      entryType: 'node',
      startTime: 0,
      duration: 112.42291688919067,
      nodeStart: 1.5486669540405273,
      v8Start: 3.308124542236328,
      bootstrapComplete: 22.103166580200195,
      environment: 12.799707889556885,
      loopStart: -1,
      loopExit: -1,
      idleTime: 0
    },
    timeOrigin: 1697039951604.925
  },
  clearImmediate: [Function: clearImmediate],
  setImmediate: [Function: setImmediate] {
    [Symbol(nodejs.util.promisify.custom)]: [Getter]
  }
}

[Ethan-Arrowood]

Why is this so painful lol

[KhafraDev]

I can't believe I forgot this, but __filename isn't a global because it wouldn't work (how would it change for every file?), cjs modules are wrapped in an iife that has __filename, __dirname, etc. So I guess node core isn't doing that for dependencies.

[KhafraDev]

I think we should just make an esbuild plugin that injects a variable into lib/fetch/index.js then we can do the typeof check

[Ethan-Arrowood]

Have you written one of those before? I have not but can start looking into it.

[KhafraDev]

I will have a PR fixing this soon

[KhafraDev]

#2341

[Ethan-Arrowood]

I found this thread: evanw/esbuild#859 (comment)

I don't love this solution mainly because I don't understand what the plugin is really doing. Would love some more input before we ship this

[Ethan-Arrowood]

Closing in favor of #2341