-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Bubble up config and block errors to the extension GUI #5990
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…ndle cases where we need error propagation, return a sum type from unrollBlocks, wrap objects in JSON.stringify to avoid displaying [object Object].
Your cubic subscription is currently inactive. Please reactivate your subscription to receive AI reviews and use cubic. |
✅ Deploy Preview for continuedev canceled.
|
…ancement/bubble-up-config-errors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks solid! Not nearly as many changes as I expected.
Also thanks for fixing the shrinking error on the icon, that has been bothering me for a bit 😅
Left a handful of nitpick comments. Most important are probably the error string copy and trying to reduce the ternaries on needErrorPropagation
…ancement/bubble-up-config-errors
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here is the error message I got for the invalid rule
Failed to parse block: [
{
"code": "too_big",
"maximum": 1,
"type": "array",
"inclusive": true,
"exact": true,
"message": "Array must contain exactly 1 element(s)",
"path": [
"rules"
]
}
]:
{"uriType":"file","filePath":"file:///Users/patrickerichsen/Git/continue-dev/continue/extensions/.continue-debug/rules/test.yaml"}
I didn't realize this on our first pass but there's still 2 problems with error bubbling here:
- For local blocks, apparently we set the
uses
clause to an object with the URI - For errors downstream of our changes here, we're still bubbling non-user friendly errors, eg my error was from here: https://github.com/continuedev/continue/blob/main/packages/config-yaml/src/load/unroll.ts#L64
Both of these are relatively easily solvable I think
- We could make a quick util fn that checks if
uses
is an object which contains afilePath
and then print that instead of the object, else just print the regularuses
- Update the handful of downstream errors in
unroll.ts
to parse out the Zod errors into something more human readable, eg here is what Claude 4 gave me
function formatZodError(error: any): string {
if (error.errors && Array.isArray(error.errors)) {
return error.errors
.map((e: any) => {
const path = e.path.length > 0 ? `${e.path.join(".")}: ` : "";
return `${path}${e.message}`;
})
.join(", ");
}
return error.message || "Validation failed";
}
export function parseBlock(configYaml: string): Block {
try {
const parsed = YAML.parse(configYaml);
const result = blockSchema.safeParse(parsed);
if (result.success) {
return result.data;
}
throw new Error(`Block validation failed: ${formatZodError(result.error)}`);
} catch (e: any) {
if (e.message.includes("Block validation failed:")) {
throw e; // Re-throw our formatted error
}
throw new Error(`Failed to parse block: ${e.message}`);
}
}
With these changes my error would look like this
Block validation failed: rules: Array must contain exactly 1 element(s):
file:///Users/patrickerichsen/Git/continue-dev/continue/extensions/.continue-debug/rules/test.yaml
Although I think the most ideal version would read
Block validation failed!
Block: file:///Users/patrickerichsen/Git/continue-dev/continue/extensions/.continue-debug/rules/test.yaml
Error: Array must contain exactly 1 element(s)
…ancement/bubble-up-config-errors
…ancement/bubble-up-config-errors
Description
Previously, errors when loading config and blocks were not being rendered. This made errors in configuration and rules difficult to catch.
Here's a motivating example:
This is syntactically valid YAML, but our rule parser will error out because it expects one rule per YAML file. These errors in semantics were difficult to catch without an error message. Now users will be able to see errors from the extension GUI.
This PR also fixes a subtle bug where exclamation triangle icon in the warning box will shrink depending on the size of the displayed error message.
Checklist
Screenshots
Tests
[ What tests were added or updated to ensure the changes work as expected? ]