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

Add bounds checking for sampleTypeIndex #449

Merged
merged 2 commits into from
Dec 8, 2023

Conversation

jlfwong
Copy link
Owner

@jlfwong jlfwong commented Dec 8, 2023

Wow this was surprising. As reported in #448, the simple.speedscope.json file failed import. This was surprising to me because there's a test that covers that file to ensure it imports correctly.

The file provided in #448, however, is from a version of speedscope from 5 years ago before the file had been pretty printed. It turns out that when this particular file is not pretty-printed, it's a schematically valid pprof profile.

The fix is to do some bounds checks and return null. After the change, the file imports as you'd expect after realizing its not actually a valid pprof profile.

Fixes #448

@coveralls
Copy link

Coverage Status

coverage: 43.041% (+0.09%) from 42.952%
when pulling 87391ee on jlfwong/add-more-error-checking-for-pprof-import
into de17f12 on main.

@jlfwong jlfwong merged commit ac4a015 into main Dec 8, 2023
8 checks passed
@jlfwong jlfwong deleted the jlfwong/add-more-error-checking-for-pprof-import branch December 8, 2023 02:43
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.

Could not import the json example into www.speedscope.app
2 participants