From b7a198cbf3e0bb41e43e52c9e58bd2b200886503 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Tue, 9 Jul 2024 15:39:07 -0400 Subject: [PATCH 1/3] Gracefully handle missing recordings.log entries --- .../src/recording/getRecordings.test.ts | 10 ++ .../shared/src/recording/getRecordings.ts | 93 ++++++++++++++++--- 2 files changed, 88 insertions(+), 15 deletions(-) diff --git a/packages/shared/src/recording/getRecordings.test.ts b/packages/shared/src/recording/getRecordings.test.ts index 304e43c2d..27d246f28 100644 --- a/packages/shared/src/recording/getRecordings.test.ts +++ b/packages/shared/src/recording/getRecordings.test.ts @@ -131,4 +131,14 @@ describe("getRecordings", () => { ]) ); }); + + it("should gracefully handle missing recording log entries", () => { + mockReadFileSync.mockReturnValue(` + {"id":"fake","kind":"addMetadata","metadata":{"processGroupId":"fake"}} + {"id":"fake","kind":"writeFinished"} + `); + + const recordings = getRecordings(); + expect(recordings).toHaveLength(0); + }); }); diff --git a/packages/shared/src/recording/getRecordings.ts b/packages/shared/src/recording/getRecordings.ts index 5b6c91373..2fa3d35d6 100644 --- a/packages/shared/src/recording/getRecordings.ts +++ b/packages/shared/src/recording/getRecordings.ts @@ -4,7 +4,7 @@ import { basename } from "path"; import { logger } from "../logger"; import { recordingLogPath } from "./config"; import { readRecordingLog } from "./readRecordingLog"; -import { LocalRecording, RECORDING_LOG_KIND } from "./types"; +import { LocalRecording, LogEntry, RECORDING_LOG_KIND } from "./types"; export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { const recordings: LocalRecording[] = []; @@ -22,7 +22,10 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { case RECORDING_LOG_KIND.addMetadata: { const { id, metadata = {} } = entry; const recording = idToRecording[id]; - assert(recording, `Recording with ID "${id}" not found`); + if (!recording) { + logNotFoundWarning(id, entries, entry); + continue; + } Object.assign(recording.metadata, metadata); @@ -58,7 +61,11 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { const { data, id } = entry; const recording = idToRecording[id]; - assert(recording, `Recording with ID "${id}" not found`); + if (!recording) { + logNotFoundWarning(id, entries, entry); + continue; + } + if (recording.crashData) { recording.crashData.push(data); } else { @@ -70,7 +77,11 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { const { id } = entry; const recording = idToRecording[id]; - assert(recording, `Recording with ID "${id}" not found`); + if (!recording) { + logNotFoundWarning(id, entries, entry); + continue; + } + recording.recordingStatus = "crashed"; break; } @@ -117,7 +128,10 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { ); const recording = idToRecording[recordingId]; - assert(recording, `Recording with ID "${recordingId}" not found`); + if (!recording) { + logNotFoundWarning(recordingId, entries, entry); + continue; + } const sourceMap = recording.metadata.sourceMaps.find( sourceMap => sourceMap.id === parentId @@ -134,7 +148,11 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { const { id } = entry; const recording = idToRecording[id]; - assert(recording, `Recording with ID "${id}" not found`); + if (!recording) { + logNotFoundWarning(id, entries, entry); + continue; + } + recording.processingStatus = "failed"; break; } @@ -142,7 +160,11 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { const { id } = entry; const recording = idToRecording[id]; - assert(recording, `Recording with ID "${id}" not found`); + if (!recording) { + logNotFoundWarning(id, entries, entry); + continue; + } + recording.processingStatus = "processed"; break; } @@ -150,7 +172,11 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { const { id } = entry; const recording = idToRecording[id]; - assert(recording, `Recording with ID "${id}" not found`); + if (!recording) { + logNotFoundWarning(id, entries, entry); + continue; + } + recording.processingStatus = "processing"; break; } @@ -158,7 +184,11 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { const { id, reason } = entry; const recording = idToRecording[id]; - assert(recording, `Recording with ID "${id}" not found`); + if (!recording) { + logNotFoundWarning(id, entries, entry); + continue; + } + recording.recordingStatus = "unusable"; recording.unusableReason = reason; break; @@ -179,7 +209,10 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { assert(targetMapURLHash, '"sourcemapAdded" entry must have a "targetMapURLHash"'); const recording = idToRecording[recordingId]; - assert(recording, `Recording with ID "${recordingId}" not found`); + if (!recording) { + logNotFoundWarning(recordingId, entries, entry); + continue; + } recording.metadata.sourceMaps.push({ id, @@ -196,7 +229,11 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { const { id } = entry; const recording = idToRecording[id]; - assert(recording, `Recording with ID "${id}" not found`); + if (!recording) { + logNotFoundWarning(id, entries, entry); + continue; + } + recording.uploadStatus = "failed"; break; } @@ -204,7 +241,11 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { const { id } = entry; const recording = idToRecording[id]; - assert(recording, `Recording with ID "${id}" not found`); + if (!recording) { + logNotFoundWarning(id, entries, entry); + continue; + } + recording.uploadStatus = "uploaded"; break; } @@ -212,7 +253,11 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { const { id } = entry; const recording = idToRecording[id]; - assert(recording, `Recording with ID "${id}" not found`); + if (!recording) { + logNotFoundWarning(id, entries, entry); + continue; + } + recording.uploadStatus = "uploading"; break; } @@ -220,7 +265,11 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { const { id, timestamp } = entry; const recording = idToRecording[id]; - assert(recording, `Recording with ID "${id}" not found`); + if (!recording) { + logNotFoundWarning(id, entries, entry); + continue; + } + recording.recordingStatus = "finished"; const startTimestamp = idToStartTimestamp[id]; @@ -233,7 +282,11 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { const { id, path, timestamp } = entry; const recording = idToRecording[id]; - assert(recording, `Recording with ID "${id}" not found`); + if (!recording) { + logNotFoundWarning(id, entries, entry); + continue; + } + recording.path = path; idToStartTimestamp[id] = timestamp; break; @@ -273,3 +326,13 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { .sort((a, b) => b.date.getTime() - a.date.getTime()) ); } + +function logNotFoundWarning(recordingId: string, entries: LogEntry[], entry: LogEntry) { + logger.error(`Recording with ID "${recordingId}" not found`, { + recordingId, + entry, + entries: JSON.stringify( + entries.filter(entry => entry.id === recordingId || entry.recordingId === recordingId) + ), + }); +} From 22afccebe13c2e973b6b7e058dabe9285898b018 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 10 Jul 2024 08:31:23 -0400 Subject: [PATCH 2/3] PR feedback --- .../shared/src/recording/getRecordings.ts | 35 +++++++++---------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/packages/shared/src/recording/getRecordings.ts b/packages/shared/src/recording/getRecordings.ts index 2fa3d35d6..0c1a26316 100644 --- a/packages/shared/src/recording/getRecordings.ts +++ b/packages/shared/src/recording/getRecordings.ts @@ -23,7 +23,7 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { const { id, metadata = {} } = entry; const recording = idToRecording[id]; if (!recording) { - logNotFoundWarning(id, entries, entry); + logNotFoundWarning(id, entry); continue; } @@ -62,7 +62,7 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { const recording = idToRecording[id]; if (!recording) { - logNotFoundWarning(id, entries, entry); + logNotFoundWarning(id, entry); continue; } @@ -78,7 +78,7 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { const recording = idToRecording[id]; if (!recording) { - logNotFoundWarning(id, entries, entry); + logNotFoundWarning(id, entry); continue; } @@ -129,7 +129,7 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { const recording = idToRecording[recordingId]; if (!recording) { - logNotFoundWarning(recordingId, entries, entry); + logNotFoundWarning(recordingId, entry); continue; } @@ -149,7 +149,7 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { const recording = idToRecording[id]; if (!recording) { - logNotFoundWarning(id, entries, entry); + logNotFoundWarning(id, entry); continue; } @@ -161,7 +161,7 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { const recording = idToRecording[id]; if (!recording) { - logNotFoundWarning(id, entries, entry); + logNotFoundWarning(id, entry); continue; } @@ -173,7 +173,7 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { const recording = idToRecording[id]; if (!recording) { - logNotFoundWarning(id, entries, entry); + logNotFoundWarning(id, entry); continue; } @@ -185,7 +185,7 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { const recording = idToRecording[id]; if (!recording) { - logNotFoundWarning(id, entries, entry); + logNotFoundWarning(id, entry); continue; } @@ -210,7 +210,7 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { const recording = idToRecording[recordingId]; if (!recording) { - logNotFoundWarning(recordingId, entries, entry); + logNotFoundWarning(recordingId, entry); continue; } @@ -230,7 +230,7 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { const recording = idToRecording[id]; if (!recording) { - logNotFoundWarning(id, entries, entry); + logNotFoundWarning(id, entry); continue; } @@ -242,7 +242,7 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { const recording = idToRecording[id]; if (!recording) { - logNotFoundWarning(id, entries, entry); + logNotFoundWarning(id, entry); continue; } @@ -254,7 +254,7 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { const recording = idToRecording[id]; if (!recording) { - logNotFoundWarning(id, entries, entry); + logNotFoundWarning(id, entry); continue; } @@ -266,7 +266,7 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { const recording = idToRecording[id]; if (!recording) { - logNotFoundWarning(id, entries, entry); + logNotFoundWarning(id, entry); continue; } @@ -283,7 +283,7 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { const recording = idToRecording[id]; if (!recording) { - logNotFoundWarning(id, entries, entry); + logNotFoundWarning(id, entry); continue; } @@ -327,12 +327,9 @@ export function getRecordings(processGroupIdFilter?: string): LocalRecording[] { ); } -function logNotFoundWarning(recordingId: string, entries: LogEntry[], entry: LogEntry) { - logger.error(`Recording with ID "${recordingId}" not found`, { +function logNotFoundWarning(recordingId: string, entry: LogEntry) { + logger.error("RecordingLog:RecordingNotFound", { recordingId, entry, - entries: JSON.stringify( - entries.filter(entry => entry.id === recordingId || entry.recordingId === recordingId) - ), }); } From f1746ca72ec61f271d429295a8a5bf3881547dc6 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 10 Jul 2024 08:32:37 -0400 Subject: [PATCH 3/3] Create tender-waves-grab.md --- .changeset/tender-waves-grab.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/tender-waves-grab.md diff --git a/.changeset/tender-waves-grab.md b/.changeset/tender-waves-grab.md new file mode 100644 index 000000000..182e83ac3 --- /dev/null +++ b/.changeset/tender-waves-grab.md @@ -0,0 +1,5 @@ +--- +"replayio": patch +--- + +Gracefully handle missing recordings.log entries