-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Allow json in workflow run's error field #12762
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
🚀 Preview Environment Ready! Your preview environment is available at: http://bore.pub:48743 This environment will automatically shut down when the PR is closed or after 5 hours. |
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.
PR Summary
Enhanced workflow error visualization by allowing JSON objects in error fields, enabling detailed error inspection while maintaining UI clarity.
- Modified
packages/twenty-front/src/modules/workflow/validation-schemas/workflowSchema.ts
to accept JSON objects in error fields instead of just strings - Added red highlighting variant in
packages/twenty-ui/src/json-visualizer
components to visually distinguish error content - Updated
packages/twenty-front/src/modules/workflow/hooks/useWorkflowRun.ts
to properly handle validation failures with detailed error objects - Improved
WorkflowRunStepOutputDetail
component to only highlight the error key in red, keeping other output data readable - Enhanced error visualization in JsonTree component with support for nested object highlighting
6 files reviewed, 4 comments
Edit PR Review Bot Settings | Greptile
@@ -6,16 +6,20 @@ import { useJsonTreeContextOrThrow } from '@ui/json-visualizer/hooks/useJsonTree | |||
import { ANIMATION } from '@ui/theme'; | |||
import { motion } from 'framer-motion'; | |||
|
|||
const StyledButton = styled(motion.button)` | |||
const StyledButton = styled(motion.button)<{ variant?: 'blue' | 'red' }>` |
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.
style: Consider using union type definition instead of inline type. Extract ArrowVariant = 'blue' | 'red'
to types.
color: ${({ theme }) => theme.font.color.tertiary}; | ||
const StyledElementsCount = styled.span<{ variant?: 'red' }>` | ||
color: ${({ theme, variant }) => | ||
variant === 'red' ? theme.font.color.danger : theme.font.color.tertiary}; | ||
`; | ||
|
||
const StyledJsonList = styled(JsonList)``.withComponent(motion.ul); |
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.
style: StyledJsonList using withComponent after styled definition is an unusual pattern. Consider using styled(motion.ul) directly
if (!isDefined(rawRecord)) { | ||
return undefined; | ||
} |
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.
logic: This check should come before the schema validation on line 22 to prevent unnecessary validation of undefined records.
if (!isDefined(rawRecord)) { | |
return undefined; | |
} | |
const { record: rawRecord } = useFindOneRecord({ | |
objectNameSingular: CoreObjectNameSingular.WorkflowRun, | |
objectRecordId: workflowRunId, | |
}); | |
if (!isDefined(rawRecord)) { | |
return undefined; | |
} | |
const { | |
success, | |
data: record, | |
error, | |
} = useMemo(() => workflowRunSchema.safeParse(rawRecord), [rawRecord]); |
const setRedHighlightingForEveryNode: GetJsonNodeHighlighting = (keyPath) => { | ||
if (keyPath === 'error') { | ||
return 'red'; | ||
} | ||
|
||
return undefined; | ||
}; |
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.
style: Consider extracting setRedHighlightingForEveryNode
to a utility function since it's a pure function that could be reused across components
We can now inspect errors even if they contain complex data as objects. Only the first line of the error is put in red.