Skip to content

Conversation

@Mercy811
Copy link
Contributor

Summary

Cherry pick #1428

Checklist

  • Does your PR title have the correct title format?
  • Does your PR have a breaking change?:

@Mercy811
Copy link
Contributor Author

Integration tests failed probably because some mocks in tests are not picked up in this feature branch. Hold this PR until they are solved

@junjie-amplitude
Copy link
Collaborator

Can we keep this change in the original branch feat-zoning-1211-plus-diagnostics-user-agent and just pre-release that branch instead? Alvin may need to continue to work on feat-zonging-1211 when he's back

@macroscopeapp
Copy link
Contributor

macroscopeapp bot commented Dec 29, 2025

Track browser user agent in diagnostics by setting AmplitudeBrowser.init to tag user_agent, and set a 10000 ms timeout for one integration Jest test in index.test.ts

Add a user_agent tag using navigator.userAgent to the diagnostics client in AmplitudeBrowser.init in browser-client.ts. Update one integration test to use a 10000 ms Jest timeout in index.test.ts.

📍Where to Start

Start with AmplitudeBrowser.init in browser-client.ts to review how the diagnostics client sets the user_agent tag.


Macroscope summarized d9c897d.

},
);
diagnosticsClient.setTag('library', `${LIBPREFIX}/${VERSION}`);
diagnosticsClient.setTag('user_agent', navigator.userAgent);
Copy link
Contributor

Choose a reason for hiding this comment

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

Accessing navigator.userAgent without a guard can crash in non‑browser contexts. Consider guarding with typeof navigator !== 'undefined' (like in Context) before setting the tag.

Suggested change
diagnosticsClient.setTag('user_agent', navigator.userAgent);
if (typeof navigator !== 'undefined') {
diagnosticsClient.setTag('user_agent', navigator.userAgent);
}

🚀 Want me to fix this? Reply ex: "fix it for me".

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants