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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

馃悰 [RUMF-1491] fix error when calling fetch with an unexpected value as first parameter #2061

Merged
merged 2 commits into from Mar 21, 2023

Conversation

BenoitZugmeyer
Copy link
Member

@BenoitZugmeyer BenoitZugmeyer commented Mar 7, 2023

Motivation

When called with anything other than Request as a first argument, fetch will use the string representation of the first parameter as an URL. For example:

fetch({ toString() { return 'toto' }}) // equivalent to fetch("toto")
fetch(null) // equivalent to fetch("null")
fetch() // equivalent to fetch("undefined")

Our fetch instrumentation does not support such usage and crashes with the following errors:

URL constructor: null is not a valid URL.
Cannot read properties of null (reading 'url')

Changes

  • consider the input argument unknown
  • make sure we don't crash with unknown values
  • make sure the correct method and URL are correctly infered from unknown values
  • make sure the requestInput is passed untouched in the event domain

Testing

  • Local
  • Staging
  • Unit
  • End to end

I have gone over the contributing documentation.

@BenoitZugmeyer BenoitZugmeyer force-pushed the benoit/fix-fetch-null-parameter branch from 31a20fe to d6c6508 Compare March 7, 2023 17:49
@codecov-commenter
Copy link

codecov-commenter commented Mar 7, 2023

Codecov Report

Merging #2061 (8592c97) into main (6c436f1) will decrease coverage by 0.13%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2061      +/-   ##
==========================================
- Coverage   93.57%   93.44%   -0.13%     
==========================================
  Files         156      156              
  Lines        5522     5522              
  Branches     1256     1256              
==========================================
- Hits         5167     5160       -7     
- Misses        355      362       +7     
Impacted Files Coverage 螖
packages/rum-core/src/domain/requestCollection.ts 100.00% <酶> (酶)
packages/core/src/browser/fetchObservable.ts 95.12% <100.00%> (酶)

... and 2 files with indirect coverage changes

馃摚 We鈥檙e building smart automated test selection to slash your CI/CD build times. Learn more

@BenoitZugmeyer BenoitZugmeyer marked this pull request as ready for review March 7, 2023 17:56
@BenoitZugmeyer BenoitZugmeyer requested a review from a team as a code owner March 7, 2023 17:56
@BenoitZugmeyer BenoitZugmeyer changed the title 馃悰 fix error when calling fetch(null) 馃悰 fix error when calling fetch with unsupported value as first parameter Mar 7, 2023
@BenoitZugmeyer BenoitZugmeyer changed the title 馃悰 fix error when calling fetch with unsupported value as first parameter 馃悰 fix error when calling fetch with an unsupported value as first parameter Mar 7, 2023
@BenoitZugmeyer BenoitZugmeyer changed the title 馃悰 fix error when calling fetch with an unsupported value as first parameter 馃悰 fix error when calling fetch with an unexpected value as first parameter Mar 7, 2023
@BenoitZugmeyer BenoitZugmeyer changed the title 馃悰 fix error when calling fetch with an unexpected value as first parameter 馃悰 [RUMF-1491] fix error when calling fetch with an unexpected value as first parameter Mar 8, 2023
packages/core/src/browser/fetchObservable.ts Outdated Show resolved Hide resolved
Copy link
Member Author

BenoitZugmeyer commented Mar 21, 2023

BenoitZugmeyer and others added 2 commits March 21, 2023 16:16
Co-authored-by: Bastien Caudan <1331991+bcaudan@users.noreply.github.com>
@BenoitZugmeyer BenoitZugmeyer force-pushed the benoit/fix-fetch-null-parameter branch from 9a6ad6a to 8592c97 Compare March 21, 2023 15:21
@BenoitZugmeyer BenoitZugmeyer merged commit 4b34567 into main Mar 21, 2023
@BenoitZugmeyer BenoitZugmeyer deleted the benoit/fix-fetch-null-parameter branch March 21, 2023 17:00
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.

None yet

3 participants