Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fix illegal invocation error on final harvest #594

Merged
merged 5 commits into from
Jun 22, 2023

Conversation

patrickhousley
Copy link
Contributor

@patrickhousley patrickhousley commented Jun 22, 2023

An issue from 1.234.0 has been addressed in which an illegal invocation error could occur during a final harvest as a result of calling addPageAction and passing window.location as an argument before a navigation.

Overview

Reverting a change from #521. Calling addPageAction and passing window.location as an argument before a navigation would result in calling .toString on the global window.location. Doing so during a page unload causes an Illegal Invocation error due to the global window.location.toString() losing it's context.

Related Issue(s)

https://issues.newrelic.com/browse/NEWRELIC-9370

Testing

Test added to ensure this issue does not happen again.

@patrickhousley patrickhousley added bug Something isn't working safe to test labels Jun 22, 2023
Copy link
Member

@metal-messiah metal-messiah left a comment

Choose a reason for hiding this comment

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

Excellent job diagnosing this

tests/specs/ins/harvesting.e2e.js Show resolved Hide resolved
tests/assets/api/addPageAction-unload.html Show resolved Hide resolved
@github-actions
Copy link

github-actions bot commented Jun 22, 2023

Static Badge

Last ran on June 21, 2023 23:20:28 CDT
Checking merge of (e968d39) into main (5175356)

@codecov
Copy link

codecov bot commented Jun 22, 2023

Codecov Report

Merging #594 (e968d39) into main (5175356) will increase coverage by 0.58%.
The diff coverage is 100.00%.

❗ Current head e968d39 differs from pull request most recent head 234b6a2. Consider uploading reports for the commit 234b6a2 to get more accurate results

@@            Coverage Diff             @@
##             main     #594      +/-   ##
==========================================
+ Coverage   67.54%   68.12%   +0.58%     
==========================================
  Files         132      129       -3     
  Lines        5907     5993      +86     
  Branches     1084     1141      +57     
==========================================
+ Hits         3990     4083      +93     
  Misses       1547     1547              
+ Partials      370      363       -7     
Flag Coverage Δ
integration-tests 84.13% <100.00%> (ø)
jest-component 21.03% <83.33%> (-0.05%) ⬇️
jest-unit 22.32% <100.00%> (+3.89%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/common/harvest/harvest.js 97.63% <100.00%> (+6.95%) ⬆️

... and 13 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@github-actions
Copy link

Asset Size Report

Merging this pull request will result in the following CDN asset size changes:

Asset Name Previous Size New Size Diff
nr-loader-spa.min 50.56 kB / 17.23 kB (gzip) 50.56 kB / 17.23 kB (gzip) -0% / -0% (gzip)
nr-loader-full.min 44.65 kB / 15.45 kB (gzip) 44.65 kB / 15.45 kB (gzip) -0% / -0% (gzip)
nr-loader-rum.min 26.48 kB / 9.7 kB (gzip) 26.48 kB / 9.7 kB (gzip) -0% / -0% (gzip)
nr-loader-spa-polyfills.min 123.08 kB / 39.36 kB (gzip) 123.08 kB / 39.36 kB (gzip) -0% / -0% (gzip)
nr-loader-full-polyfills.min 115.6 kB / 37.42 kB (gzip) 115.6 kB / 37.42 kB (gzip) -0% / -0% (gzip)
nr-loader-rum-polyfills.min 94.55 kB / 31.23 kB (gzip) 94.55 kB / 31.23 kB (gzip) -0% / +0.01% (gzip)
nr-loader-worker.min 41.2 kB / 14.19 kB (gzip) 41.2 kB / 14.19 kB (gzip) -0% / -0% (gzip)

Merging this pull request will result in the following NPM package consumer size changes:

Asset Name Previous Size New Size Diff
Browser Agent 50.69 kB / 17.11 kB (gzip) 50.69 kB / 17.11 kB (gzip) -0% / -0% (gzip)
Custom Lite Agent 26.63 kB / 9.62 kB (gzip) 26.63 kB / 9.62 kB (gzip) -0% / -0% (gzip)
Custom Pro Agent 44.71 kB / 15.27 kB (gzip) 44.71 kB / 15.27 kB (gzip) -0% / -0% (gzip)
Custom SPA Agent 50.58 kB / 17.08 kB (gzip) 50.58 kB / 17.08 kB (gzip) -0% / -0% (gzip)
Worker Agent 303.58 kB / 94.03 kB (gzip) 303.65 kB / 94.05 kB (gzip) +0.02% / +0.02% (gzip)
Other Standard CDN Assets

Released Assets

Asset Name Asset Size
recorder.1b18459f.min.js 166.07 kB / 52.32 kB (gzip)
spa-aggregate.084ee148.min.js 20.93 kB / 7.42 kB (gzip)
page_view_timing-aggregate.ba48a30f.min.js 14.97 kB / 5.43 kB (gzip)
860.79ccac1e.min.js 14.35 kB / 5.33 kB (gzip)
session_trace-aggregate.f2588580.min.js 12.24 kB / 4.52 kB (gzip)
page_view_event-aggregate.b566f156.min.js 11.07 kB / 4.17 kB (gzip)
jserrors-aggregate.6787bff1.min.js 9.58 kB / 3.67 kB (gzip)
metrics-aggregate.3728fba2.min.js 8.56 kB / 2.93 kB (gzip)
148.9b104f6e.min.js 7.85 kB / 3.26 kB (gzip)
session_replay-aggregate.05ab3aeb.min.js 7.28 kB / 2.75 kB (gzip)
ajax-aggregate.d6946c36.min.js 7.13 kB / 3.06 kB (gzip)
compressor.ae9f91a8.min.js 7.09 kB / 3.57 kB (gzip)
page_action-aggregate.5ff09456.min.js 4.73 kB / 1.92 kB (gzip)
async-api.44b145a1.min.js 2.92 kB / 1.46 kB (gzip)
session-manager.2a64278a.min.js 1.47 kB / 740 B (gzip)
lazy-feature-loader.0ba331d7.min.js 1.17 kB / 499 B (gzip)

Built Assets

Asset Name Asset Size
recorder.1b18459f.min.js 166.07 kB / 52.32 kB (gzip)
spa-aggregate.084ee148.min.js 20.93 kB / 7.42 kB (gzip)
page_view_timing-aggregate.ba48a30f.min.js 14.97 kB / 5.43 kB (gzip)
860.79ccac1e.min.js 14.35 kB / 5.33 kB (gzip)
session_trace-aggregate.f2588580.min.js 12.24 kB / 4.52 kB (gzip)
page_view_event-aggregate.b566f156.min.js 11.07 kB / 4.17 kB (gzip)
jserrors-aggregate.6787bff1.min.js 9.58 kB / 3.67 kB (gzip)
metrics-aggregate.3728fba2.min.js 8.56 kB / 2.93 kB (gzip)
148.85ae9674.min.js 7.91 kB / 3.29 kB (gzip)
session_replay-aggregate.05ab3aeb.min.js 7.28 kB / 2.75 kB (gzip)
ajax-aggregate.d6946c36.min.js 7.13 kB / 3.06 kB (gzip)
compressor.ae9f91a8.min.js 7.09 kB / 3.57 kB (gzip)
page_action-aggregate.5ff09456.min.js 4.73 kB / 1.92 kB (gzip)
async-api.44b145a1.min.js 2.92 kB / 1.46 kB (gzip)
session-manager.2a64278a.min.js 1.47 kB / 740 B (gzip)
lazy-feature-loader.0ba331d7.min.js 1.17 kB / 499 B (gzip)
Other Polyfill CDN Assets

Released Assets

Asset Name Asset Size
recorder.1b18459f-es5.min.js 166.76 kB / 52.35 kB (gzip)
nr-polyfills.min.js 52.15 kB / 17.95 kB (gzip)
session_trace-aggregate.919b51bb-es5.min.js 32.23 kB / 8.5 kB (gzip)
compressor.79fb47d9-es5.min.js 30.02 kB / 11.29 kB (gzip)
spa-aggregate.5d42071b-es5.min.js 24.02 kB / 8.11 kB (gzip)
page_view_timing-aggregate.37dab318-es5.min.js 18.67 kB / 6.17 kB (gzip)
session_replay-aggregate.5925bc6e-es5.min.js 18.57 kB / 6.35 kB (gzip)
173.b8688949-es5.min.js 17.69 kB / 6.2 kB (gzip)
page_view_event-aggregate.c52d6109-es5.min.js 12.59 kB / 4.78 kB (gzip)
jserrors-aggregate.21b50a10-es5.min.js 12.38 kB / 4.42 kB (gzip)
ajax-aggregate.efddc02b-es5.min.js 10.58 kB / 3.78 kB (gzip)
646.cadfd5a4-es5.min.js 10.4 kB / 4.08 kB (gzip)
metrics-aggregate.c503733c-es5.min.js 9.91 kB / 3.35 kB (gzip)
page_action-aggregate.9576f950-es5.min.js 7.33 kB / 2.64 kB (gzip)
async-api.b0969e41-es5.min.js 4.16 kB / 1.99 kB (gzip)
session-manager.ae4f9cf3-es5.min.js 1.7 kB / 785 B (gzip)
lazy-feature-loader.fe02c936-es5.min.js 1.2 kB / 509 B (gzip)

Built Assets

Asset Name Asset Size
recorder.1b18459f-es5.min.js 166.76 kB / 52.35 kB (gzip)
nr-polyfills.min.js 52.15 kB / 17.95 kB (gzip)
session_trace-aggregate.919b51bb-es5.min.js 32.23 kB / 8.5 kB (gzip)
compressor.79fb47d9-es5.min.js 30.02 kB / 11.29 kB (gzip)
spa-aggregate.5d42071b-es5.min.js 24.02 kB / 8.11 kB (gzip)
page_view_timing-aggregate.37dab318-es5.min.js 18.67 kB / 6.17 kB (gzip)
session_replay-aggregate.5925bc6e-es5.min.js 18.57 kB / 6.35 kB (gzip)
173.b8688949-es5.min.js 17.69 kB / 6.2 kB (gzip)
page_view_event-aggregate.c52d6109-es5.min.js 12.59 kB / 4.78 kB (gzip)
jserrors-aggregate.21b50a10-es5.min.js 12.38 kB / 4.42 kB (gzip)
ajax-aggregate.efddc02b-es5.min.js 10.58 kB / 3.78 kB (gzip)
646.dc447cab-es5.min.js 10.45 kB / 4.1 kB (gzip)
metrics-aggregate.c503733c-es5.min.js 9.91 kB / 3.35 kB (gzip)
page_action-aggregate.9576f950-es5.min.js 7.33 kB / 2.64 kB (gzip)
async-api.b0969e41-es5.min.js 4.16 kB / 1.99 kB (gzip)
session-manager.ae4f9cf3-es5.min.js 1.7 kB / 785 B (gzip)
lazy-feature-loader.fe02c936-es5.min.js 1.2 kB / 509 B (gzip)

@bjfield bjfield changed the title fix: final harvest illegal invocation fix: Fix illegal invocation error on final harvest Jun 22, 2023
@patrickhousley patrickhousley merged commit de7049f into main Jun 22, 2023
@patrickhousley patrickhousley deleted the fix-unload-illegal-invocation branch June 22, 2023 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants