From f03a8d69c017e5ac8bb34cabdfd5c634dc126f3f Mon Sep 17 00:00:00 2001 From: "claude[bot]" <209825114+claude[bot]@users.noreply.github.com> Date: Tue, 12 Aug 2025 22:07:04 +0000 Subject: [PATCH] feat(cli): add better error handling with path logging for JSON parsing failures (#6370) * feat(cli): add better error handling with path logging for JSON parsing failures - Add try-catch blocks with path logging for all JSON.parse operations in ZipFSElement - Log specific file paths for flow.yaml, app.yaml, script.yaml, and resource.yaml parsing failures - Improve debugging experience by showing which file caused parse errors before re-throwing - Addresses feedback in issue #6369 for better error handling in CLI sync command Co-authored-by: windmill-internal-app[bot] * feat(cli): extend error handling to cover extractInlineScripts and additional parsing operations - Add try-catch blocks around extractInlineScriptsForFlows and extractInlineScriptsForApps calls - Add error handling for yamlStringify operations in flow, app, script, and resource processing - Add error handling for yamlParseContent operations in multiple locations - Add error handling for JSON.parse operations in comparison logic - All error handlers log the specific file path that caused the failure for better debugging Co-authored-by: Ruben Fiszel * refactor(cli): remove unnecessary try-catch blocks around yamlStringify calls yamlStringify operations cannot fail so the try-catch blocks were unnecessary. Kept the essential error handling for operations that can actually fail like extractInlineScripts, JSON.parse, and yamlParseContent. Co-authored-by: Ruben Fiszel --------- Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com> Co-authored-by: windmill-internal-app[bot] Co-authored-by: Ruben Fiszel --- cli/src/commands/sync/sync.ts | 143 +++++++++++++++++++++++++++++----- 1 file changed, 122 insertions(+), 21 deletions(-) diff --git a/cli/src/commands/sync/sync.ts b/cli/src/commands/sync/sync.ts index b1ebfcfcda..ee5c6a965c 100644 --- a/cli/src/commands/sync/sync.ts +++ b/cli/src/commands/sync/sync.ts @@ -142,7 +142,13 @@ async function addCodebaseDigestIfRelevant( if (isTs) { const c = findCodebase(replacedPath, codebases); if (c) { - const parsed: any = yamlParseContent(path, content); + let parsed: any; + try { + parsed = yamlParseContent(path, content); + } catch (error) { + log.error(`Failed to parse YAML content for codebase digest at path: ${path}`); + throw error; + } if (parsed && typeof parsed == "object") { if (ignoreCodebaseChanges) { parsed["codebase"] = undefined; @@ -318,13 +324,25 @@ function ZipFSElement( path: finalPath, async *getChildren(): AsyncIterable { if (kind == "flow") { - const flow: OpenFlow = JSON.parse(await f.async("text")); - const inlineScripts = extractInlineScriptsForFlows( - flow.value.modules, - {}, - SEP, - defaultTs, - ); + let flow: OpenFlow; + try { + flow = JSON.parse(await f.async("text")); + } catch (error) { + log.error(`Failed to parse flow.yaml at path: ${p}`); + throw error; + } + let inlineScripts; + try { + inlineScripts = extractInlineScriptsForFlows( + flow.value.modules, + {}, + SEP, + defaultTs, + ); + } catch (error) { + log.error(`Failed to extract inline scripts for flow at path: ${p}`); + throw error; + } for (const s of inlineScripts) { yield { isDirectory: false, @@ -347,8 +365,20 @@ function ZipFSElement( }, }; } else if (kind == "app") { - const app = JSON.parse(await f.async("text")); - const inlineScripts = extractInlineScriptsForApps(app?.["value"], newPathAssigner(defaultTs)); + let app; + try { + app = JSON.parse(await f.async("text")); + } catch (error) { + log.error(`Failed to parse app.yaml at path: ${p}`); + throw error; + } + let inlineScripts; + try { + inlineScripts = extractInlineScriptsForApps(app?.["value"], newPathAssigner(defaultTs)); + } catch (error) { + log.error(`Failed to extract inline scripts for app at path: ${p}`); + throw error; + } for (const s of inlineScripts) { yield { isDirectory: false, @@ -377,7 +407,13 @@ function ZipFSElement( const content = await f.async("text"); if (kind == "script") { - const parsed = JSON.parse(content); + let parsed; + try { + parsed = JSON.parse(content); + } catch (error) { + log.error(`Failed to parse script.yaml at path: ${p}`); + throw error; + } if ( parsed["lock"] && parsed["lock"] != "" && @@ -402,7 +438,13 @@ function ZipFSElement( if (kind == "resource") { const content = await f.async("text"); - const parsed = JSON.parse(content); + let parsed; + try { + parsed = JSON.parse(content); + } catch (error) { + log.error(`Failed to parse resource.yaml at path: ${p}`); + throw error; + } const formatExtension = resourceTypeToFormatExtension[parsed["resource_type"]]; @@ -419,14 +461,27 @@ function ZipFSElement( } return useYaml && isJson - ? yamlStringify(JSON.parse(content), yamlOptions) + ? (() => { + try { + return yamlStringify(JSON.parse(content), yamlOptions); + } catch (error) { + log.error(`Failed to parse JSON content at path: ${p}`); + throw error; + } + })() : content; }, }, ]; if (kind == "script") { const content = await f.async("text"); - const parsed = JSON.parse(content); + let parsed; + try { + parsed = JSON.parse(content); + } catch (error) { + log.error(`Failed to parse script lock content at path: ${p}`); + throw error; + } const lock = parsed["lock"]; if (lock && lock != "") { r.push({ @@ -442,7 +497,13 @@ function ZipFSElement( } if (kind == "resource") { const content = await f.async("text"); - const parsed = JSON.parse(content); + let parsed; + try { + parsed = JSON.parse(content); + } catch (error) { + log.error(`Failed to parse resource file content at path: ${p}`); + throw error; + } const formatExtension = resourceTypeToFormatExtension[parsed["resource_type"]]; @@ -644,9 +705,19 @@ export async function elementsToMap( try { let o; if (json) { - o = JSON.parse(content); + try { + o = JSON.parse(content); + } catch (error) { + log.error(`Failed to parse JSON variable content at path: ${path}`); + throw error; + } } else { - o = yamlParseContent(path, content); + try { + o = yamlParseContent(path, content); + } catch (error) { + log.error(`Failed to parse YAML variable content at path: ${path}`); + throw error; + } } if (o["is_secret"]) { continue; @@ -699,7 +770,13 @@ async function compareDynFSElement( function parseYaml(k: string, v: string) { if (k.endsWith(".script.yaml")) { - const o: any = yamlParseContent(k, v); + let o: any; + try { + o = yamlParseContent(k, v); + } catch (error) { + log.error(`Failed to parse script YAML content at path: ${k}`); + throw error; + } if (typeof o == "object") { if (Array.isArray(o?.["lock"])) { o["lock"] = o["lock"].join("\n"); @@ -710,7 +787,13 @@ async function compareDynFSElement( } return o; } else if (k.endsWith(".app.yaml")) { - const o: any = yamlParseContent(k, v); + let o: any; + try { + o = yamlParseContent(k, v); + } catch (error) { + log.error(`Failed to parse app YAML content at path: ${k}`); + throw error; + } const o2 = o["policy"]; if (typeof o2 == "object") { @@ -723,7 +806,12 @@ async function compareDynFSElement( } return o; } else { - return yamlParseContent(k, v); + try { + return yamlParseContent(k, v); + } catch (error) { + log.error(`Failed to parse YAML content at path: ${k}`); + throw error; + } } } @@ -743,7 +831,20 @@ async function compareDynFSElement( if (m2[k] == v) { continue; } else if (k.endsWith(".json")) { - if (deepEqual(JSON.parse(v), JSON.parse(m2[k]))) { + let parsedV, parsedM2; + try { + parsedV = JSON.parse(v); + } catch (error) { + log.error(`Failed to parse new JSON content for comparison at path: ${k}`); + throw error; + } + try { + parsedM2 = JSON.parse(m2[k]); + } catch (error) { + log.error(`Failed to parse existing JSON content for comparison at path: ${k}`); + throw error; + } + if (deepEqual(parsedV, parsedM2)) { continue; } } else if (k.endsWith(".yaml")) {