Skip to content

Commit 59c58e4

Browse files
authored
[LOGS-2] Improve error handling on save and check step (#1209)
## TL;DR * Fixes bug during 'Check step' where `executeStep` mutation still runs if `updateStep` failed * Removed unnecessary toast notifications and throw directly from the onError handler in ApolloProvider ## How to test? Happy flow - [ ] Save step still works as expected - [ ] Check step still executes both `updateStep` and `executeStep` mutations as expected Unhappy flow - [ ] When checking step `updateStep` mutation failure should throw error and NOT execute `executeStep` - [ ] Should only have 1 error toast ## Screenshots ![Screenshot 2025-09-15 at 12.18.00 PM.png](https://app.graphite.dev/user-attachments/assets/5f206a12-f72f-4ef4-b72b-854713c7991d.png)
1 parent 8746502 commit 59c58e4

File tree

4 files changed

+52
-49
lines changed

4 files changed

+52
-49
lines changed

packages/backend/src/graphql/__tests__/mutations/update-step.itest.ts

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { NotFoundError } from 'objection'
12
import { beforeEach, describe, expect, it, vi } from 'vitest'
23

34
import { BadUserInputError } from '@/errors/graphql-errors'
@@ -19,23 +20,35 @@ describe('updateStep mutation', () => {
1920
const patchFlowLastUpdatedSpy = vi.fn().mockResolvedValue({})
2021

2122
// Helper to create step mock with flow
22-
const createStepMock = (stepData: any = {}) => ({
23-
findOne: vi.fn().mockResolvedValue(
24-
stepData === null
25-
? null
26-
: {
27-
id: mockStepId,
28-
key: 'sendTransactionalEmail',
29-
appKey: 'postman',
30-
status: 'completed',
31-
flow: {
32-
id: mockFlowId,
33-
},
34-
patchFlowLastUpdated: patchFlowLastUpdatedSpy,
35-
...stepData,
36-
},
37-
),
38-
})
23+
const createStepMock = (stepData: any = {}) => {
24+
const throwIfNotFoundMock = vi.fn().mockImplementation(async (options) => {
25+
const result =
26+
stepData === null
27+
? null
28+
: {
29+
id: mockStepId,
30+
key: 'sendTransactionalEmail',
31+
appKey: 'postman',
32+
status: 'completed',
33+
flow: {
34+
id: mockFlowId,
35+
},
36+
patchFlowLastUpdated: patchFlowLastUpdatedSpy,
37+
...stepData,
38+
}
39+
40+
if (result === null) {
41+
throw new NotFoundError(options?.message || 'Step not found')
42+
}
43+
return result
44+
})
45+
46+
return {
47+
findOne: vi.fn().mockReturnValue({
48+
throwIfNotFound: throwIfNotFoundMock,
49+
}),
50+
}
51+
}
3952

4053
// Helper to create connection mock
4154
const createConnectionMock = (connectionData: any = null) => ({
@@ -49,7 +62,7 @@ describe('updateStep mutation', () => {
4962
) => {
5063
context.currentUser.$relatedQuery = vi
5164
.fn()
52-
.mockImplementation((relation) => {
65+
.mockImplementation((relation, _trx) => {
5366
if (relation === 'steps') {
5467
return createStepMock(stepData)
5568
}
@@ -165,7 +178,7 @@ describe('updateStep mutation', () => {
165178
// Override only the connections query
166179
setupRelatedQueryMock(undefined, null)
167180

168-
await expect(updateStep(null, { input }, context)).rejects.toThrow(
181+
await expect(updateStep(null, { input }, context)).rejects.toThrowError(
169182
BadUserInputError,
170183
)
171184
expect(patchAndFetchByIdSpy).not.toHaveBeenCalled()
@@ -181,7 +194,7 @@ describe('updateStep mutation', () => {
181194
setupRelatedQueryMock(null, { id: mockConnectionId })
182195

183196
await expect(updateStep(null, { input }, context)).rejects.toThrow(
184-
BadUserInputError,
197+
NotFoundError,
185198
)
186199
expect(patchAndFetchByIdSpy).not.toHaveBeenCalled()
187200
})

packages/backend/src/graphql/mutations/update-step.ts

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,13 @@ const updateStep: MutationResolvers['updateStep'] = async (
1212
const { input } = params
1313

1414
const step = await Step.transaction(async (trx) => {
15-
const step = await context.currentUser.$relatedQuery('steps', trx).findOne({
16-
'steps.id': input.id,
17-
flow_id: input.flow.id,
18-
})
19-
20-
if (!step) {
21-
throw new BadUserInputError('Step not found')
22-
}
15+
const step = await context.currentUser
16+
.$relatedQuery('steps', trx)
17+
.findOne({
18+
'steps.id': input.id,
19+
flow_id: input.flow.id,
20+
})
21+
.throwIfNotFound({ message: 'Step not found' })
2322

2423
if (input.connection.id) {
2524
// if connectionId is specified, verify that the connection exists and belongs to the user

packages/frontend/src/components/FlowSubstep/index.tsx

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -96,29 +96,17 @@ function FlowSubstep(props: FlowSubstepProps): JSX.Element {
9696
setIsSaving(true)
9797
const currentStep = formContext.getValues() as IStep
9898
const isSubStepValid = validateSubstep(substep, currentStep)
99-
const result = await onUpdateStep({
99+
await onUpdateStep({
100100
...currentStep,
101101
status: isSubStepValid ? 'completed' : 'incomplete',
102102
})
103-
104-
if (!result) {
105-
throw new Error('Failed to save step')
106-
}
107103
} catch (error) {
108-
toast({
109-
title: 'Error saving step',
110-
description:
111-
error instanceof Error
112-
? error.message
113-
: 'An unexpected error occurred',
114-
status: 'error',
115-
duration: 5000,
116-
isClosable: true,
117-
})
104+
console.error('Error saving step', error)
105+
throw error // Re-throw the error so calling functions know saveStep failed
118106
} finally {
119107
setIsSaving(false)
120108
}
121-
}, [formContext, onUpdateStep, substep, toast])
109+
}, [formContext, onUpdateStep, substep])
122110

123111
const handleSave = useCallback(async () => {
124112
await saveStep()
@@ -132,10 +120,14 @@ function FlowSubstep(props: FlowSubstepProps): JSX.Element {
132120

133121
const handleSaveAndTest = useCallback(
134122
async (testRunMetadata?: Record<string, unknown>) => {
135-
await saveStep()
136-
await executeTestStep(testRunMetadata)
137-
if (!isIfThenStep(step)) {
138-
onTestResultOpen()
123+
try {
124+
await saveStep()
125+
await executeTestStep(testRunMetadata)
126+
if (!isIfThenStep(step)) {
127+
onTestResultOpen()
128+
}
129+
} catch (error) {
130+
console.error('Error saving and test step')
139131
}
140132
},
141133
[saveStep, executeTestStep, step, onTestResultOpen],

packages/frontend/src/contexts/Editor.tsx

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,6 @@ export const EditorProvider = ({
326326
const [executeStep, { loading: isTestExecuting }] = useMutation(
327327
EXECUTE_STEP,
328328
{
329-
context: { autoSnackbar: false },
330329
awaitRefetchQueries: true,
331330
refetchQueries: [GET_TEST_EXECUTION_STEPS, GET_FLOW],
332331
update(cache, { data }) {

0 commit comments

Comments
 (0)