Fail loudly when profile is imported with unmatched open/close events (#273)

Before this change, profiles like those in #272 would import but would display misleading data. Let's fail hard instead.

Fixes #272

sample/profiles/speedscope/invalid/incomplete-trace.json

@@ -0,0 +1,16 @@
+{
+  "$schema": "https://www.speedscope.app/file-format-schema.json",
+  "shared": {
+    "frames": [{"name": "A"}]
+  },
+  "profiles": [
+    {
+      "type": "evented",
+      "name": "p1",
+      "unit": "none",
+      "startValue": 0,
+      "endValue": 100,
+      "events": [{"type": "O", "frame": 0, "at": 0}]
+    }
+  ]
+}

sample/profiles/speedscope/invalid/out-of-order-events.json

@@ -0,0 +1,25 @@
+{
+  "$schema": "https://www.speedscope.app/file-format-schema.json",
+  "shared": {
+    "frames": [{"name": "A"}, {"name": "B"}]
+  },
+  "profiles": [
+    {
+      "type": "evented",
+      "name": "p1",
+      "unit": "none",
+      "startValue": 0,
+      "endValue": 100,
+      "events": [
+        {"type": "O", "frame": 0, "at": 0},
+        {"type": "C", "frame": 0, "at": 1},
+        {"type": "O", "frame": 1, "at": 2},
+        {"type": "O", "frame": 0, "at": 2},
+        {"type": "O", "frame": 0, "at": 3},
+        {"type": "C", "frame": 0, "at": 4},
+        {"type": "C", "frame": 1, "at": 4},
+        {"type": "C", "frame": 0, "at": 5}
+      ]
+    }
+  ]
+}

src/lib/__snapshots__/file-format.test.ts.snap

@@ -211,3 +211,7 @@ Object {
 exports[`importSpeedscopeProfiles 0.6.0 multiple profiles: indexToView 1`] = `1`;
 
 exports[`importSpeedscopeProfiles 0.6.0 multiple profiles: profileGroup.name 1`] = `"Two Samples"`;
+
+exports[`importSpeedscopeProfiles invalid due to incomplete trace 1`] = `"Tried to complete profile construction with a non-empty stack"`;
+
+exports[`importSpeedscopeProfiles invalid due to out of order events 1`] = `"Tried to leave frame \\"B\\" while frame \\"A\\" was at the top at 4"`;

src/lib/file-format.test.ts

@@ -1,4 +1,4 @@
-import {checkProfileSnapshot} from './test-utils'
+import {checkProfileSnapshot, expectImportFailure} from './test-utils'
 
 describe('importSpeedscopeProfiles', () => {
   test('0.0.1 evented profile', async () => {
@@ -12,4 +12,13 @@ describe('importSpeedscopeProfiles', () => {
   test('0.6.0 multiple profiles', async () => {
     await checkProfileSnapshot('./sample/profiles/speedscope/0.6.0/two-sampled.speedscope.json')
   })
+
+  test('invalid due to out of order events', async () => {
+    // See: https://github.com/jlfwong/speedscope/issues/272
+    await expectImportFailure('./sample/profiles/speedscope/invalid/out-of-order-events.json')
+  })
+
+  test('invalid due to incomplete trace', async () => {
+    await expectImportFailure('./sample/profiles/speedscope/invalid/incomplete-trace.json')
+  })
 })

src/lib/profile.ts

@@ -603,6 +603,13 @@ export class CallTreeProfileBuilder extends Profile {
         throw new Error(`Trying to leave a ${frame.key} before any have been entered`)
       }
       leavingStackTop.freeze()
+
+      if (leavingStackTop.frame.key !== frame.key) {
+        throw new Error(
+          `Tried to leave frame "${frame.name}" while frame "${leavingStackTop.frame.name}" was at the top at ${value}`,
+        )
+      }
+
       const delta = value - this.lastValue
       if (delta > 0) {
         this.samples.push(leavingStackTop)

src/lib/test-utils.ts

@@ -97,3 +97,14 @@ export async function checkProfileSnapshot(filepath: string) {
   const reexported = exportProfileGroup(reimportedGroup)
   expect(exported).toEqual(reexported)
 }
+
+export async function expectImportFailure(filepath: string) {
+  const buffer = fs.readFileSync(filepath)
+  const arrayBuffer = buffer.buffer.slice(buffer.byteOffset, buffer.byteOffset + buffer.byteLength)
+  try {
+    await importProfilesFromArrayBuffer(path.basename(filepath), arrayBuffer)
+    fail('Expected import to fail but it succeeded')
+  } catch (error) {
+    expect(error.message).toMatchSnapshot()
+  }
+}