Skip to content

fix(browser-detector): ensure Node.js v21+ is not detected as browser#6271

Merged
pichlermarc merged 13 commits intoopen-telemetry:mainfrom
fiyinfoluwa001:fix/browser-detector-navigator-check
Jan 16, 2026
Merged

fix(browser-detector): ensure Node.js v21+ is not detected as browser#6271
pichlermarc merged 13 commits intoopen-telemetry:mainfrom
fiyinfoluwa001:fix/browser-detector-navigator-check

Conversation

@fiyinfoluwa001
Copy link
Copy Markdown
Contributor

Fixes #6244 Description: > Modern Node.js and Bun runtimes now define a global navigator object. This caused the BrowserDetector to incorrectly identify these environments as browsers. Changes:

Updated isBrowser check to explicitly verify that process.versions.node and Bun.version are undefined.

Added a regression test case in BrowserDetector.test.ts that runs in Node.js to verify an empty resource is returned even when navigator is present.

@fiyinfoluwa001 fiyinfoluwa001 requested a review from a team as a code owner January 8, 2026 14:26
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla Bot commented Jan 8, 2026

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: overbalance / name: Jared Freeze (d9473ec)

@fiyinfoluwa001
Copy link
Copy Markdown
Contributor Author

fiyinfoluwa001 commented Jan 8, 2026

Uploading Screenshot from 2026-01-08 15-26-45.png…

@overbalance overbalance added the browser Browser-specific additions or benefits label Jan 8, 2026
Copy link
Copy Markdown
Contributor

@overbalance overbalance left a comment

Choose a reason for hiding this comment

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

This code doesn't detect browsers and misses some less popular runtimes and environments. I would recommend the inverse:

typeof window !== 'undefined' && typeof window.document !== 'undefined'

@fiyinfoluwa001
Copy link
Copy Markdown
Contributor Author

This makes more sense . I'll update the logic to use feature detection for window and document and verify that the Node.js regression test still passes.

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 8, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.45%. Comparing base (0a9b614) to head (d9473ec).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6271      +/-   ##
==========================================
+ Coverage   95.41%   95.45%   +0.04%     
==========================================
  Files         317      317              
  Lines        9597     9597              
  Branches     2220     2221       +1     
==========================================
+ Hits         9157     9161       +4     
+ Misses        440      436       -4     
Files with missing lines Coverage Δ
...ntelemetry-browser-detector/src/BrowserDetector.ts 26.66% <100.00%> (+13.33%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@overbalance
Copy link
Copy Markdown
Contributor

overbalance commented Jan 9, 2026

@fiyinfoluwa001 While testing, I didn't realize I could push to your branch. 🤷 My mistake.

I created a helper function (used elsewhere in OTel) called describeNode and updated the test. I can rollback if you like but I think the new comparison handles more environments.

@overbalance overbalance dismissed their stale review January 9, 2026 00:57

Removing my review because I unintentionally added code.

@fiyinfoluwa001
Copy link
Copy Markdown
Contributor Author

Thank you for the help with describeNode and the environment check refactor! I’ve pulled the changes locally. I appreciate the guidance on following the project's testing standards.

@overbalance overbalance enabled auto-merge January 14, 2026 18:37
@overbalance overbalance disabled auto-merge January 14, 2026 18:38
@pichlermarc pichlermarc enabled auto-merge January 15, 2026 09:01
@overbalance overbalance requested a review from a team as a code owner January 16, 2026 01:35
@pichlermarc pichlermarc added this pull request to the merge queue Jan 16, 2026
Merged via the queue into open-telemetry:main with commit c62d120 Jan 16, 2026
27 checks passed
@otelbot
Copy link
Copy Markdown
Contributor

otelbot Bot commented Jan 16, 2026

Thank you for your contribution @fiyinfoluwa001! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

browser Browser-specific additions or benefits

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BrowserDetector does not accurately detect browsers

3 participants