diff --git a/src/profile-logic/process-profile.ts b/src/profile-logic/process-profile.ts index 24846ec17b..d83c75a447 100644 --- a/src/profile-logic/process-profile.ts +++ b/src/profile-logic/process-profile.ts @@ -73,6 +73,7 @@ import type { GeckoMetaMarkerSchema, GeckoStaticFieldSchemaData, GeckoMarkers, + GeckoMarkerStack, GeckoMarkerStruct, GeckoMarkerTuple, GeckoFrameStruct, @@ -562,6 +563,20 @@ function _processStackTable( return stackIndexOffset; } +/** + * We expect a captured backtrace here, with a samples table. But the "stack" key + * isn't reserved for that: some markers store an unrelated value (e.g. Log markers + * from the test harness put a textual stack trace string there). Such a value has + * no `samples`, so checking for it both selects real backtraces and keeps one bad + * marker from failing the whole profile. A non-backtrace stack is left in place to + * be handled by the marker schema like any other field. + */ +function _payloadHasStack( + data: MarkerPayload_Gecko +): data is MarkerPayload_Gecko & { stack: GeckoMarkerStack } { + return 'stack' in data && !!data.stack?.samples?.data.length; +} + /** * Convert stack field to cause field for the given payload. A cause field includes * the thread ID (tid), an IndexIntoStackTable, and the time the stack was captured. @@ -573,7 +588,7 @@ function _convertStackToCause( data: MarkerPayload_Gecko, stackIndexOffset: IndexIntoStackTable ) { - if ('stack' in data && data.stack && data.stack.samples.data.length > 0) { + if (_payloadHasStack(data)) { const { stack, ...newData } = data; const stackIndex = stack.samples.data[0][stack.samples.schema.stack]; const time = stack.samples.data[0][stack.samples.schema.time]; @@ -599,7 +614,7 @@ function _convertPayloadStackToIndex( if (!data) { return null; } - if ('stack' in data && data.stack && data.stack.samples.data.length > 0) { + if (_payloadHasStack(data)) { const { samples } = data.stack; const geckoStackIndex = samples.data[0][samples.schema.stack]; if (geckoStackIndex !== null) { diff --git a/src/test/unit/marker-data.test.ts b/src/test/unit/marker-data.test.ts index 0e49044eb5..80c8c41ab0 100644 --- a/src/test/unit/marker-data.test.ts +++ b/src/test/unit/marker-data.test.ts @@ -100,6 +100,44 @@ describe('Derive markers from Gecko phase markers', function () { ]); }); + it('keeps a non-backtrace string stack instead of crashing', function () { + // The Gecko "stack" key isn't reserved for captured backtraces. Log markers + // emitted by the test harness store a textual JS stack trace string there. + // Such a value has no samples table, so it must be left untouched (not turned + // into a cause) rather than crashing the whole profile during processing. + const stackString = + '_abort_failed_test@head.js:938:20\ndo_report_result@head.js:1053:5\n'; + const { markers } = setupWithTestDefinedMarkers([ + { + startTime: 5, + endTime: null, + phase: INSTANT, + data: { + type: 'Log', + level: 1, + message: 'Unexpected exception NS_ERROR_ABORT', + stack: stackString, + } as any, + }, + ]); + + expect(markers).toEqual([ + { + category: 0, + data: { + type: 'Log', + level: 1, + message: 'Unexpected exception NS_ERROR_ABORT', + stack: stackString, + }, + end: null, + name: 'TestDefinedMarker', + start: 5, + threadId: null, + }, + ]); + }); + it('matches an IntervalStart and IntervalEnd marker', function () { const { markers } = setupWithTestDefinedMarkers([ {