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(instrumentation-user-interaction): addEventListener throws when calling with useCapture = null #1045

Merged
merged 4 commits into from
Jun 20, 2022

Conversation

t2t2
Copy link
Contributor

@t2t2 t2t2 commented Jun 1, 2022

Which problem is this PR solving?

When addEventListener is called with useCapture = null (technically against the spec but that stops nobody), event listener instrumentation throws and breaks apps

chrome:
TypeError: Cannot read properties of null (reading 'once')
    at addEventListenerPatched (https://tpmfzt.csb.app/node_modules/@opentelemetry/instrumentation-user-interaction/build/esm/instrumentation.js:180:66)

firefox:
useCapture is null
  at addEventListenerPatched@https://tpmfzt.csb.app/node_modules/@opentelemetry/instrumentation-user-interaction/build/esm/instrumentation.js:180:20

Repo: https://codesandbox.io/s/otel-user-interaction-event-capture-null-tpmfzt?file=/src/listener-null.js

Example of this error spotted in the wild: react-onclickoutside

image

Caused by typeof null === 'object'

Short description of the changes

  • Extra check for value being true-ish

@t2t2 t2t2 requested a review from a team June 1, 2022 12:28
@github-actions github-actions bot requested a review from obecny June 1, 2022 12:28
@codecov
Copy link

codecov bot commented Jun 1, 2022

Codecov Report

Merging #1045 (63ed1bf) into main (69efe64) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #1045   +/-   ##
=======================================
  Coverage   95.91%   95.91%           
=======================================
  Files          13       13           
  Lines         856      856           
  Branches      178      178           
=======================================
  Hits          821      821           
  Misses         35       35           
Impacted Files Coverage Δ
...umentation-user-interaction/src/instrumentation.ts 92.28% <100.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.

4 participants