-
Notifications
You must be signed in to change notification settings - Fork 117
fix: harden canvas node rendering and workflow mapper preparation #3899
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
Closed
Closed
Changes from all commits
Commits
Show all changes
18 commits
Select commit
Hold shift + click to select a range
658f4e0
fix(workflowv2): harden node mapper preparation
forestileao c021639
fix(canvas): isolate block render failures
forestileao 82a51b3
chore(ui): update eslint budget baseline
forestileao f207e2b
fix
forestileao 90fb991
Merge branch 'main' into fix/nodes-errors-should-not-break-the-canvas
forestileao c48f66b
enforce a better message
forestileao 11bb12e
address issues
forestileao d936794
refactor(workflowv2): rename lib files to kebab-case
forestileao 2f61c6a
fix(canvas): remove internal node name props
forestileao c6f9b85
refactor(canvas): dedupe default block wiring
forestileao 476ab84
fix
forestileao 5de6371
address unused exports
forestileao 93b4166
prevent flood
forestileao 398e840
address
forestileao c272972
address
forestileao b0c07e8
address
forestileao 5efd0d4
fix conflict
forestileao f656683
fix conflict
forestileao File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,20 +1,18 @@ | ||
| { | ||
| "maxAllowedTotalIssues": 1270, | ||
| "maxAllowedTotalIssues": 1226, | ||
| "maxAllowedByRule": { | ||
| "@typescript-eslint/no-explicit-any": 359, | ||
| "complexity": 273, | ||
| "@typescript-eslint/consistent-type-imports": 200, | ||
| "max-statements": 103, | ||
| "max-lines-per-function": 97, | ||
| "@typescript-eslint/no-non-null-asserted-optional-chain": 97, | ||
| "@typescript-eslint/no-explicit-any": 344, | ||
| "complexity": 270, | ||
| "@typescript-eslint/consistent-type-imports": 194, | ||
| "max-statements": 102, | ||
| "max-lines-per-function": 96, | ||
| "@typescript-eslint/no-non-null-asserted-optional-chain": 87, | ||
| "react-hooks/exhaustive-deps": 59, | ||
| "max-lines": 26, | ||
| "react-refresh/only-export-components": 25, | ||
| "max-depth": 11, | ||
| "max-params": 8, | ||
| "jsx-a11y/label-has-associated-control": 6, | ||
| "react-hooks/rules-of-hooks": 4, | ||
| "no-case-declarations": 2 | ||
| "max-params": 6 | ||
| }, | ||
| "updatedAt": "2026-04-01T15:16:27.934Z" | ||
| "updatedAt": "2026-04-01T22:39:48.685Z" | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| export const CANVAS_NODE_FALLBACK_MESSAGE = "Can't display"; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| export type UnknownRecord = Record<string, unknown>; | ||
|
|
||
| export function isRecord(value: unknown): value is UnknownRecord { | ||
| return typeof value === "object" && value !== null; | ||
| } | ||
forestileao marked this conversation as resolved.
Show resolved
Hide resolved
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,150 @@ | ||
| import { QueryClient } from "@tanstack/react-query"; | ||
| import { beforeEach, describe, expect, it, vi } from "vitest"; | ||
| import type { ComponentsComponent, ComponentsNode } from "@/api-client"; | ||
| import type { CustomFieldRenderer } from "./mappers/types"; | ||
| import * as mappers from "./mappers"; | ||
| import { createSafeCustomFieldRenderer } from "./mappers/safeMappers"; | ||
| import { prepareComponentBaseNode, prepareTriggerNode } from "./lib/canvas-node-preparation"; | ||
| import { renderWorkflowNodeCustomField } from "./lib/render-workflow-node-custom-field"; | ||
|
|
||
| type FallbackComponentData = { | ||
| renderFallback?: { | ||
| source: string; | ||
| message: string; | ||
| }; | ||
| component: { | ||
| error?: string; | ||
| emptyStateProps?: { | ||
| title?: string; | ||
| }; | ||
| }; | ||
| }; | ||
|
|
||
| function makeNode(overrides: Partial<ComponentsNode> = {}): ComponentsNode { | ||
| return { | ||
| id: "node-1", | ||
| name: "Broken Component", | ||
| type: "TYPE_COMPONENT", | ||
| position: { x: 10, y: 20 }, | ||
| component: { | ||
| name: "approval", | ||
| }, | ||
| configuration: {}, | ||
| ...overrides, | ||
| } as ComponentsNode; | ||
| } | ||
|
|
||
| function makeComponent(overrides: Partial<ComponentsComponent> = {}): ComponentsComponent { | ||
| return { | ||
| name: "approval", | ||
| label: "Approval", | ||
| icon: "hand", | ||
| color: "orange", | ||
| outputChannels: [{ name: "default" }], | ||
| ...overrides, | ||
| } as ComponentsComponent; | ||
| } | ||
|
|
||
| function makeTriggerNode(overrides: Partial<ComponentsNode> = {}): ComponentsNode { | ||
| return { | ||
| id: "trigger-1", | ||
| name: "Incoming Event", | ||
| type: "TYPE_TRIGGER", | ||
| position: { x: 0, y: 0 }, | ||
| trigger: { | ||
| name: "webhook", | ||
| }, | ||
| configuration: {}, | ||
| ...overrides, | ||
| } as ComponentsNode; | ||
| } | ||
|
|
||
| describe("workflow node preparation resilience", () => { | ||
| beforeEach(() => { | ||
| vi.restoreAllMocks(); | ||
| }); | ||
|
|
||
| it("returns a fallback canvas node when component preparation fails", () => { | ||
| vi.spyOn(mappers, "getComponentAdditionalDataBuilder").mockReturnValue({ | ||
| buildAdditionalData: () => { | ||
| throw new Error("builder failed"); | ||
| }, | ||
| }); | ||
| vi.spyOn(mappers, "getComponentBaseMapper").mockReturnValue({ | ||
| props: () => { | ||
| throw new Error("mapper failed"); | ||
| }, | ||
| subtitle: () => "", | ||
| getExecutionDetails: () => ({}), | ||
| }); | ||
|
|
||
| const result = prepareComponentBaseNode({ | ||
| nodes: [makeNode()], | ||
| node: makeNode(), | ||
| components: [makeComponent()], | ||
| nodeExecutionsMap: {}, | ||
| nodeQueueItemsMap: {}, | ||
| workflowId: "canvas-1", | ||
| queryClient: new QueryClient(), | ||
| organizationId: "org-1", | ||
| }); | ||
|
|
||
| const fallbackData = result.data as unknown as FallbackComponentData; | ||
|
|
||
| expect(fallbackData.renderFallback).toEqual({ | ||
| source: "mapper", | ||
| message: "Can't display", | ||
| }); | ||
| expect(fallbackData.component.error).toBeUndefined(); | ||
| expect(fallbackData.component.emptyStateProps?.title).toBe("Can't display"); | ||
| }); | ||
|
|
||
| it("returns null when a custom field renderer throws so sidebar rendering stays alive", () => { | ||
| const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {}); | ||
| const renderer = createSafeCustomFieldRenderer( | ||
| { | ||
| render: () => { | ||
| throw new Error("custom field failed"); | ||
| }, | ||
| } satisfies CustomFieldRenderer, | ||
| "approval", | ||
| ); | ||
|
|
||
| const result = renderWorkflowNodeCustomField({ | ||
| renderer, | ||
| node: makeNode(), | ||
| }); | ||
|
|
||
| expect(result).toBeNull(); | ||
| expect(consoleSpy).toHaveBeenCalledWith( | ||
| expect.stringContaining('Custom field renderer "approval" threw in render()'), | ||
| expect.any(Error), | ||
| ); | ||
| consoleSpy.mockRestore(); | ||
| }); | ||
|
|
||
| it("keeps trigger error and warning precedence on node state only", () => { | ||
| vi.spyOn(mappers, "getTriggerRenderer").mockReturnValue({ | ||
| getTriggerProps: () => ({ | ||
| title: "Webhook", | ||
| iconSlug: "bolt", | ||
| metadata: [], | ||
| error: "renderer error", | ||
| warning: "renderer warning", | ||
| }), | ||
| getRootEventValues: () => ({}), | ||
| getTitleAndSubtitle: () => ({ title: "Event", subtitle: "" }), | ||
| }); | ||
|
|
||
| const result = prepareTriggerNode( | ||
| makeTriggerNode(), | ||
| [{ name: "webhook", label: "Webhook", icon: "bolt" }] as never, | ||
| {}, | ||
| ); | ||
|
|
||
| const triggerData = result.data as { trigger: { error?: string; warning?: string } }; | ||
|
|
||
| expect(triggerData.trigger.error).toBeUndefined(); | ||
| expect(triggerData.trigger.warning).toBeUndefined(); | ||
| }); | ||
| }); |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Weird that we have a dedicated file for this and not for other files. Why?