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: property access on undefined exit span #179

Merged
merged 1 commit into from
Jan 18, 2024

Conversation

johann-crabnebula
Copy link
Contributor

This basically fixes the error getting thrown on spans that don't have an exit yet.

Even though I have to say that the underlying code is really wonky in the first place (I added the question mark to "restore the original behavior":

The original line looked like this:

const width = scaleToMax([span.exits[i] - enter], allExits - allEnters)[0];

If we the span.exits array is empty it will pretty much resolve to something like:

undefined - number = NaN

And then it will

scaleToMax(NaN, number);
// -> 
(NaN / number) * 100 = NaN

Which looks hella ugly and should be revised in the future but does not throw an error.

The updated code:

const exited = span.exits[i]?.timestamp;

const width = scaleToMax(
    [exited - entered.timestamp],
    allExits - allEnters
)[0];

Knowing that exits can be empty it is easy to spot the previous error. When exits are empty we call the property .timestamp on undefined which is not allowed.

Copy link

netlify bot commented Jan 17, 2024

Deploy Preview for cn-devtools-app ready!

Name Link
🔨 Latest commit 06d98ba
🔍 Latest deploy log https://app.netlify.com/sites/cn-devtools-app/deploys/65a93a1caad74b0008b71bfb
😎 Deploy Preview https://deploy-preview-179--cn-devtools-app.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Jan 17, 2024

Deploy Preview for cn-devtools-website canceled.

Name Link
🔨 Latest commit 06d98ba
🔍 Latest deploy log https://app.netlify.com/sites/cn-devtools-website/deploys/65a93a1c4d12410008721c76

CrabNejonas
CrabNejonas previously approved these changes Jan 18, 2024
@johann-crabnebula johann-crabnebula merged commit 048985c into main Jan 18, 2024
24 checks passed
@johann-crabnebula johann-crabnebula deleted the fix/span-timestamp branch January 18, 2024 15:30
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