Support importing unterminated JSON in simple cases (#208)
This PR introduces support for importing JSON based profiles that are missing a terminating `]` (and possibly have an extraneous `,`). This is similar to #202, but takes a much more targeted and simple approach. I'm confident that this approach is sufficient because this is exactly what `chrome://tracing` does: https://github.com/catapult-project/catapult/blob/27e047e0494df162022be6aa8a8862742a270232/tracing/tracing/extras/importer/trace_event_importer.html#L197-L208 Fixes #204
This commit is contained in:
Jamie Wong
@@ -164,6 +164,174 @@ exports[`importTraceEvents multiprocess: indexToView 1`] = `0`;
|
||||
|
||||
exports[`importTraceEvents multiprocess: profileGroup.name 1`] = `"multiprocess.json"`;
|
||||
|
||||
exports[`importTraceEvents partial json import 1`] = `
|
||||
Object {
|
||||
"frames": Array [
|
||||
Frame {
|
||||
"col": undefined,
|
||||
"file": undefined,
|
||||
"key": "alpha",
|
||||
"line": undefined,
|
||||
"name": "alpha",
|
||||
"selfWeight": 2,
|
||||
"totalWeight": 14,
|
||||
},
|
||||
Frame {
|
||||
"col": undefined,
|
||||
"file": undefined,
|
||||
"key": "beta",
|
||||
"line": undefined,
|
||||
"name": "beta",
|
||||
"selfWeight": 3,
|
||||
"totalWeight": 12,
|
||||
},
|
||||
Frame {
|
||||
"col": undefined,
|
||||
"file": undefined,
|
||||
"key": "gamma {\\"detail\\":\\"foobar\\"}",
|
||||
"line": undefined,
|
||||
"name": "gamma {\\"detail\\":\\"foobar\\"}",
|
||||
"selfWeight": 5,
|
||||
"totalWeight": 5,
|
||||
},
|
||||
Frame {
|
||||
"col": undefined,
|
||||
"file": undefined,
|
||||
"key": "epsilon",
|
||||
"line": undefined,
|
||||
"name": "epsilon",
|
||||
"selfWeight": 4,
|
||||
"totalWeight": 4,
|
||||
},
|
||||
],
|
||||
"name": "pid 0, tid 0",
|
||||
"stacks": Array [
|
||||
"alpha 1.00µs",
|
||||
"alpha;beta 1.00µs",
|
||||
"alpha;beta;gamma {\\"detail\\":\\"foobar\\"} 5.00µs",
|
||||
"alpha;beta;epsilon 4.00µs",
|
||||
"alpha;beta 2.00µs",
|
||||
"alpha 1.00µs",
|
||||
],
|
||||
}
|
||||
`;
|
||||
|
||||
exports[`importTraceEvents partial json import trailing comma 1`] = `
|
||||
Object {
|
||||
"frames": Array [
|
||||
Frame {
|
||||
"col": undefined,
|
||||
"file": undefined,
|
||||
"key": "alpha",
|
||||
"line": undefined,
|
||||
"name": "alpha",
|
||||
"selfWeight": 2,
|
||||
"totalWeight": 14,
|
||||
},
|
||||
Frame {
|
||||
"col": undefined,
|
||||
"file": undefined,
|
||||
"key": "beta",
|
||||
"line": undefined,
|
||||
"name": "beta",
|
||||
"selfWeight": 3,
|
||||
"totalWeight": 12,
|
||||
},
|
||||
Frame {
|
||||
"col": undefined,
|
||||
"file": undefined,
|
||||
"key": "gamma {\\"detail\\":\\"foobar\\"}",
|
||||
"line": undefined,
|
||||
"name": "gamma {\\"detail\\":\\"foobar\\"}",
|
||||
"selfWeight": 5,
|
||||
"totalWeight": 5,
|
||||
},
|
||||
Frame {
|
||||
"col": undefined,
|
||||
"file": undefined,
|
||||
"key": "epsilon",
|
||||
"line": undefined,
|
||||
"name": "epsilon",
|
||||
"selfWeight": 4,
|
||||
"totalWeight": 4,
|
||||
},
|
||||
],
|
||||
"name": "pid 0, tid 0",
|
||||
"stacks": Array [
|
||||
"alpha 1.00µs",
|
||||
"alpha;beta 1.00µs",
|
||||
"alpha;beta;gamma {\\"detail\\":\\"foobar\\"} 5.00µs",
|
||||
"alpha;beta;epsilon 4.00µs",
|
||||
"alpha;beta 2.00µs",
|
||||
"alpha 1.00µs",
|
||||
],
|
||||
}
|
||||
`;
|
||||
|
||||
exports[`importTraceEvents partial json import trailing comma: indexToView 1`] = `0`;
|
||||
|
||||
exports[`importTraceEvents partial json import trailing comma: profileGroup.name 1`] = `"simple-partial-trailing-comma.json"`;
|
||||
|
||||
exports[`importTraceEvents partial json import whitespace padding 1`] = `
|
||||
Object {
|
||||
"frames": Array [
|
||||
Frame {
|
||||
"col": undefined,
|
||||
"file": undefined,
|
||||
"key": "alpha",
|
||||
"line": undefined,
|
||||
"name": "alpha",
|
||||
"selfWeight": 2,
|
||||
"totalWeight": 14,
|
||||
},
|
||||
Frame {
|
||||
"col": undefined,
|
||||
"file": undefined,
|
||||
"key": "beta",
|
||||
"line": undefined,
|
||||
"name": "beta",
|
||||
"selfWeight": 3,
|
||||
"totalWeight": 12,
|
||||
},
|
||||
Frame {
|
||||
"col": undefined,
|
||||
"file": undefined,
|
||||
"key": "gamma {\\"detail\\":\\"foobar\\"}",
|
||||
"line": undefined,
|
||||
"name": "gamma {\\"detail\\":\\"foobar\\"}",
|
||||
"selfWeight": 5,
|
||||
"totalWeight": 5,
|
||||
},
|
||||
Frame {
|
||||
"col": undefined,
|
||||
"file": undefined,
|
||||
"key": "epsilon",
|
||||
"line": undefined,
|
||||
"name": "epsilon",
|
||||
"selfWeight": 4,
|
||||
"totalWeight": 4,
|
||||
},
|
||||
],
|
||||
"name": "pid 0, tid 0",
|
||||
"stacks": Array [
|
||||
"alpha 1.00µs",
|
||||
"alpha;beta 1.00µs",
|
||||
"alpha;beta;gamma {\\"detail\\":\\"foobar\\"} 5.00µs",
|
||||
"alpha;beta;epsilon 4.00µs",
|
||||
"alpha;beta 2.00µs",
|
||||
"alpha 1.00µs",
|
||||
],
|
||||
}
|
||||
`;
|
||||
|
||||
exports[`importTraceEvents partial json import whitespace padding: indexToView 1`] = `0`;
|
||||
|
||||
exports[`importTraceEvents partial json import whitespace padding: profileGroup.name 1`] = `"simple-partial-whitespace.json"`;
|
||||
|
||||
exports[`importTraceEvents partial json import: indexToView 1`] = `0`;
|
||||
|
||||
exports[`importTraceEvents partial json import: profileGroup.name 1`] = `"simple-partial.json"`;
|
||||
|
||||
exports[`importTraceEvents simple 1`] = `
|
||||
Object {
|
||||
"frames": Array [
|
||||
|
||||
+21
-1
@@ -71,6 +71,26 @@ function toGroup(profile: Profile | null): ProfileGroup | null {
|
||||
return {name: profile.getName(), indexToView: 0, profiles: [profile]}
|
||||
}
|
||||
|
||||
function fixUpJSON(content: string): string {
|
||||
// This code is similar to the code from here:
|
||||
// https://github.com/catapult-project/catapult/blob/27e047e0494df162022be6aa8a8862742a270232/tracing/tracing/extras/importer/trace_event_importer.html#L197-L208
|
||||
//
|
||||
// If the event data begins with a [, then we know it should end with a ]. The
|
||||
// reason we check for this is because some tracing implementations cannot
|
||||
// guarantee that a ']' gets written to the trace file. So, we are forgiving
|
||||
// and if this is obviously the case, we fix it up before throwing the string
|
||||
// at JSON.parse.
|
||||
//
|
||||
content = content.trim()
|
||||
if (content[0] === '[') {
|
||||
content = content.replace(/,\s*$/, '')
|
||||
if (content[content.length - 1] !== ']') {
|
||||
content += ']'
|
||||
}
|
||||
}
|
||||
return content
|
||||
}
|
||||
|
||||
async function _importProfileGroup(dataSource: ProfileDataSource): Promise<ProfileGroup | null> {
|
||||
const fileName = await dataSource.name()
|
||||
|
||||
@@ -116,7 +136,7 @@ async function _importProfileGroup(dataSource: ProfileDataSource): Promise<Profi
|
||||
// Second pass: Try to guess what file format it is based on structure
|
||||
let parsed: any
|
||||
try {
|
||||
parsed = JSON.parse(contents)
|
||||
parsed = JSON.parse(fixUpJSON(contents))
|
||||
} catch (e) {}
|
||||
if (parsed) {
|
||||
if (parsed['$schema'] === 'https://www.speedscope.app/file-format-schema.json') {
|
||||
|
||||
@@ -11,3 +11,15 @@ test('importTraceEvents simple object', async () => {
|
||||
test('importTraceEvents multiprocess', async () => {
|
||||
await checkProfileSnapshot('./sample/profiles/trace-event/multiprocess.json')
|
||||
})
|
||||
|
||||
test('importTraceEvents partial json import', async () => {
|
||||
await checkProfileSnapshot('./sample/profiles/trace-event/simple-partial.json')
|
||||
})
|
||||
|
||||
test('importTraceEvents partial json import trailing comma', async () => {
|
||||
await checkProfileSnapshot('./sample/profiles/trace-event/simple-partial-trailing-comma.json')
|
||||
})
|
||||
|
||||
test('importTraceEvents partial json import whitespace padding', async () => {
|
||||
await checkProfileSnapshot('./sample/profiles/trace-event/simple-partial-whitespace.json')
|
||||
})
|
||||
|
||||
@@ -285,13 +285,9 @@ function isTraceEventObject(
|
||||
export function isTraceEventFormatted(
|
||||
rawProfile: any,
|
||||
): rawProfile is {traceEvents: TraceEvent[]} | TraceEvent[] {
|
||||
// We're only going to suppor the JSON formatted profiles for now.
|
||||
// We're only going to support the JSON formatted profiles for now.
|
||||
// The spec also discusses support for data embedded in ftrace supported data: https://lwn.net/Articles/365835/.
|
||||
|
||||
// TODO(jlfwong): The spec also specifies that it's valid for the trace to not contain a terminating `]`.
|
||||
// That complicates things a bit for us, so let's just ignore that for now until someone writes in with a
|
||||
// bug report from real data.
|
||||
|
||||
return isTraceEventObject(rawProfile) || isTraceEventList(rawProfile)
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user