Skip to content

EMBR-10483: Switch to a buffered POST beacon for JS errors#82

Merged
joseph-embrace merged 4 commits into
mainfrom
jwynn/feat/js-error-beacon
May 3, 2026
Merged

EMBR-10483: Switch to a buffered POST beacon for JS errors#82
joseph-embrace merged 4 commits into
mainfrom
jwynn/feat/js-error-beacon

Conversation

@joseph-embrace

Copy link
Copy Markdown
Contributor

@joseph-embrace
joseph-embrace marked this pull request as ready for review April 27, 2026 01:20
@joseph-embrace
joseph-embrace force-pushed the jwynn/feat/js-error-beacon branch from 39e688e to ac9d046 Compare April 30, 2026 02:33
Comment thread src/lux.ts
// Return the connection type based on Network Information API.
// Note this API is in flux.
function connectionType() {
function connectionType(): string | undefined {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Originally the only change here was an explicit return type, but I felt the need to refactor the function body as well. navigator.connection is only ever slow-2g, 2g, 3g, or 4g. The SDK turns these into "friendly" values by upper-casing them, with the exception being "Slow 2G". The previous logic was overly complicated for such a simple transform.

Comment thread src/transport.ts
@@ -0,0 +1,15 @@
const sendBeaconFallback = (url: string, data: string) => {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Extracted this from beacon.ts since it's now shared between two beacons.

expect(nt.requestStart).toBeGreaterThan(0);
// requestStart can floor to 0 on fast localhost connections where the
// sub-millisecond time between navigationStart and requestStart is rounded down
expect(nt.requestStart).toBeGreaterThanOrEqual(0);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Unrelated but hopefully fixes a flaky integration test that was blocking the PR.

Comment thread src/config.ts Outdated
return {
allowEmptyPostBeacon: getProperty(obj, "allowEmptyPostBeacon", false),
auto: autoMode,
errorBeaconDelay: getProperty(obj, "errorBeaconDelay", 200),

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Note this is similar to interactionBeaconDelay below.

@joseph-embrace
joseph-embrace force-pushed the jwynn/feat/js-error-beacon branch from 832ecb1 to ee33a18 Compare April 30, 2026 04:03
Comment thread src/performance.ts

export function navigationType() {
export function navigationType(): number | undefined {
if (performance.navigation && typeof performance.navigation.type !== "undefined") {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

probably a separate PR but just noting this has been deprecated: https://developer.mozilla.org/en-US/docs/Web/API/PerformanceNavigation/type

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@jpmunz yeah there's quite a bit of old code in this SDK. Some of it is intentional to support old browsers, but I am definitely leaning towards just removing it these days. I don't think anyone will mind if IE11 doesn't have a navigation type :)

@joseph-embrace
joseph-embrace force-pushed the jwynn/feat/js-error-beacon branch from 6cf3605 to 7823e0c Compare April 30, 2026 21:55
@joseph-embrace
joseph-embrace force-pushed the jwynn/feat/js-error-beacon branch from 7823e0c to 717485a Compare April 30, 2026 22:32
@joseph-embrace
joseph-embrace merged commit d770021 into main May 3, 2026
4 checks passed
@joseph-embrace
joseph-embrace deleted the jwynn/feat/js-error-beacon branch May 3, 2026 20:33
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.

2 participants