Parse and store SDK validation errors in state#173
Conversation
Signed-off-by: handreyrc <handrey.cunha@gmail.com>
Signed-off-by: handreyrc <handrey.cunha@gmail.com>
✅ Deploy Preview for swf-editor ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR enhances how SDK validation failures are represented by parsing SDK validation error messages into structured objects, updating the editor store/types accordingly, and adding tests to cover the new behavior.
Changes:
- Introduce
ValidationError/SdkErrorand parse SDK validation output into structured errors. - Update store/context and error-page rendering to handle non-
Errorvalidation entries safely. - Add fixtures + integration/UI tests (with snapshots) for validation error parsing and fallback messaging.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/serverless-workflow-diagram-editor/src/core/workflowSdk.ts | Adds ValidationError/SdkError, parseValidationErrorMessage, and updates validation flow to return parsed errors. |
| packages/serverless-workflow-diagram-editor/src/store/DiagramEditorContext.tsx | Updates context errors type to SdkError[]. |
| packages/serverless-workflow-diagram-editor/src/diagram-editor/error-pages/ParsingErrorPage.tsx | Guards YAML exception checks with err instanceof Error for mixed error arrays. |
| packages/serverless-workflow-diagram-editor/tests/core/workflowSdk.integration.test.ts | Adds tests for parsing validation errors and updates invalid-workflow expectations. |
| packages/serverless-workflow-diagram-editor/tests/core/snapshots/workflowSdk.integration.test.ts.snap | Updates snapshots for new parsed validation error output. |
| packages/serverless-workflow-diagram-editor/tests/diagram-editor/error-pages/ParsingErrorPage.test.tsx | Adds tests ensuring ValidationError entries fall back to default error copy. |
| packages/serverless-workflow-diagram-editor/tests/fixtures/workflows.ts | Adds YAML fixture intended to trigger validation errors. |
| packages/serverless-workflow-diagram-editor/tests/react-flow/diagram/Diagram.test.tsx | Minor adjustment to ReactFlow mock signature. |
| .changeset/parse-sdk-validation-errors.md | Changeset documenting the new behavior as a minor bump. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: handreyrc <handrey.cunha@gmail.com>
| } | ||
|
|
||
| // Validate all required fields are non-empty | ||
| if (!taskId || !errorType || !errorMessage || !objectStr) { |
There was a problem hiding this comment.
Looks like the taskId is a path but I think the path can be empty if there is an error in top level, like if document is missing? This check would mean we miss those errors?
There was a problem hiding this comment.
Missing Document session:
In this case, it causes a parser error that is later handled by the error page, it does not get to the validation.
Missing properties in Document session:
- /document | #/properties/document/required | must have required property 'namespace' | {"missingProperty":"namespace"}
This one I checked before submitting the PR, as you can see it meets the 4 fields standard so no issues here.
Wrong DSL version:
'Workflow' is invalid - The DSL version of the workflow '1.0.0' doesn't match the supported version of the SDK '1.0.3'.
This one is problem, so I changed the ValidationError type and validation error parser to handle this sort of message too.
It should be fixed now!
| do: [], | ||
| }); | ||
|
|
||
| export const VALID_WORKFLOW_WITH_VALIDATION_ERRORS_YAML = ` |
There was a problem hiding this comment.
I was a little confused by this name :) Just a small suggestionMaybe PARSEABLE_INVALID_WORKFLOW_YAML`?
| expect(errors[0].object).toEqual({}); | ||
| }); | ||
|
|
||
| it("rejects array JSON and falls back to empty object", () => { |
There was a problem hiding this comment.
Maybe create an it.each for tests asserting {} to reduce repetition
Signed-off-by: handreyrc <handrey.cunha@gmail.com>
Signed-off-by: handreyrc <handrey.cunha@gmail.com>
Closes #96
Summary
This PR introtuces means to handle and parse validation errors into an array and keep errors up-to-date it in the store.
e.g.
'Workflow' is invalid: 'Workflow' is invalid:
- /do/0/checkup | #/oneOf/0/required | must have required property 'call' | {"missingProperty":"call"}
- /do/0/checkup | #/oneOf/0/required | must have required property 'call' | {"missingProperty":"call"}
Changes