Skip to content

Commit bd5b89b

Browse files
abhinavDhulipalaabhinavDhulipala
committed
unify error handeling across dg tool
1 parent 0eca5e8 commit bd5b89b

10 files changed

Lines changed: 173 additions & 113 deletions

File tree

‎apps/sim/tools/dagster/get_run.ts‎

Lines changed: 30 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,18 @@
1+
import { dagsterUnionErrorMessage, parseDagsterGraphqlResponse } from '@/tools/dagster/graphql'
12
import type { DagsterGetRunParams, DagsterGetRunResponse } from '@/tools/dagster/types'
23
import type { ToolConfig } from '@/tools/types'
34

5+
/** Fields selected on `runOrError` when the union resolves to `Run`. */
6+
interface DagsterGetRunGraphqlRun {
7+
runId: string
8+
jobName: string | null
9+
status: string
10+
startTime: number | null
11+
endTime: number | null
12+
runConfigYaml: string | null
13+
tags: Array<{ key: string; value: string }> | null
14+
}
15+
416
const GET_RUN_QUERY = `
517
query GetRun($runId: ID!) {
618
runOrError(runId: $runId) {
@@ -16,7 +28,8 @@ const GET_RUN_QUERY = `
1628
value
1729
}
1830
}
19-
... on RunNotFoundError {
31+
... on Error {
32+
__typename
2033
message
2134
}
2235
}
@@ -66,33 +79,29 @@ export const getRunTool: ToolConfig<DagsterGetRunParams, DagsterGetRunResponse>
6679
},
6780

6881
transformResponse: async (response: Response) => {
69-
const data = await response.json()
82+
const data = await parseDagsterGraphqlResponse<{ runOrError?: unknown }>(response)
7083

71-
if (!response.ok) {
72-
throw new Error(data.errors?.[0]?.message || 'Dagster GraphQL request failed')
73-
}
84+
const raw = data.data?.runOrError
85+
if (!raw || typeof raw !== 'object') throw new Error('Unexpected response from Dagster')
7486

75-
if (data.errors?.length) {
76-
throw new Error(data.errors[0].message)
87+
if (!('runId' in raw) || typeof (raw as { runId: unknown }).runId !== 'string') {
88+
throw new Error(
89+
dagsterUnionErrorMessage(raw as { message?: string }, 'Run not found or Dagster error')
90+
)
7791
}
7892

79-
const result = data.data?.runOrError
80-
if (!result) throw new Error('Unexpected response from Dagster')
81-
82-
if (result.message && !result.runId) {
83-
throw new Error(result.message)
84-
}
93+
const run = raw as DagsterGetRunGraphqlRun
8594

8695
return {
8796
success: true,
8897
output: {
89-
runId: result.runId,
90-
jobName: result.jobName ?? null,
91-
status: result.status,
92-
startTime: result.startTime ?? null,
93-
endTime: result.endTime ?? null,
94-
runConfigYaml: result.runConfigYaml ?? null,
95-
tags: result.tags ?? null,
98+
runId: run.runId,
99+
jobName: run.jobName ?? null,
100+
status: run.status,
101+
startTime: run.startTime ?? null,
102+
endTime: run.endTime ?? null,
103+
runConfigYaml: run.runConfigYaml ?? null,
104+
tags: run.tags ?? null,
96105
},
97106
}
98107
},
@@ -105,6 +114,7 @@ export const getRunTool: ToolConfig<DagsterGetRunParams, DagsterGetRunResponse>
105114
jobName: {
106115
type: 'string',
107116
description: 'Name of the job this run belongs to',
117+
optional: true,
108118
},
109119
status: {
110120
type: 'string',

‎apps/sim/tools/dagster/graphql.ts‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/**
2+
* Parses a Dagster GraphQL JSON body and throws if the HTTP status is not OK or the payload
3+
* contains top-level GraphQL errors.
4+
*
5+
* Field errors should be requested with `... on Error { __typename message }` (or at least
6+
* `message`) so union failures are not returned as empty objects.
7+
*/
8+
export async function parseDagsterGraphqlResponse<TData extends Record<string, unknown>>(
9+
response: Response
10+
): Promise<{ data?: TData }> {
11+
let payload: {
12+
data?: TData
13+
errors?: ReadonlyArray<{ message?: string }>
14+
}
15+
try {
16+
payload = (await response.json()) as {
17+
data?: TData
18+
errors?: ReadonlyArray<{ message?: string }>
19+
}
20+
} catch {
21+
throw new Error('Invalid JSON response from Dagster')
22+
}
23+
if (!response.ok) {
24+
throw new Error(payload.errors?.[0]?.message || 'Dagster GraphQL request failed')
25+
}
26+
if (payload.errors?.length) {
27+
throw new Error(payload.errors[0]?.message ?? 'Dagster GraphQL request failed')
28+
}
29+
return { data: payload.data }
30+
}
31+
32+
/**
33+
* Message from a field that includes `... on Error { message }`, or a fallback when the
34+
* payload is not a GraphQL `Error` type with a string message.
35+
*/
36+
export function dagsterUnionErrorMessage(
37+
result: { message?: string } | undefined,
38+
fallback: string
39+
): string {
40+
return typeof result?.message === 'string' ? result.message : fallback
41+
}

‎apps/sim/tools/dagster/launch_run.ts‎

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { dagsterUnionErrorMessage, parseDagsterGraphqlResponse } from '@/tools/dagster/graphql'
12
import type { DagsterLaunchRunParams, DagsterLaunchRunResponse } from '@/tools/dagster/types'
23
import type { ToolConfig } from '@/tools/types'
34

@@ -40,6 +41,7 @@ function buildLaunchRunMutation(hasConfig: boolean, hasTags: boolean) {
4041
}
4142
}
4243
... on Error {
44+
__typename
4345
message
4446
}
4547
}
@@ -142,15 +144,7 @@ export const launchRunTool: ToolConfig<DagsterLaunchRunParams, DagsterLaunchRunR
142144
},
143145

144146
transformResponse: async (response: Response) => {
145-
const data = await response.json()
146-
147-
if (!response.ok) {
148-
throw new Error(data.errors?.[0]?.message || 'Dagster GraphQL request failed')
149-
}
150-
151-
if (data.errors?.length) {
152-
throw new Error(data.errors[0].message)
153-
}
147+
const data = await parseDagsterGraphqlResponse<{ launchRun?: unknown }>(response)
154148

155149
const result = data.data?.launchRun as LaunchRunResult | undefined
156150
if (!result) throw new Error('Unexpected response from Dagster')
@@ -162,7 +156,9 @@ export const launchRunTool: ToolConfig<DagsterLaunchRunParams, DagsterLaunchRunR
162156
}
163157
}
164158

165-
throw new Error(`${result.type}: ${result.message ?? 'Launch run failed'}`)
159+
throw new Error(
160+
`${result.type}: ${dagsterUnionErrorMessage(result, 'Launch run failed')}`
161+
)
166162
},
167163

168164
outputs: {

‎apps/sim/tools/dagster/list_jobs.ts‎

Lines changed: 23 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { dagsterUnionErrorMessage, parseDagsterGraphqlResponse } from '@/tools/dagster/graphql'
12
import type { DagsterBaseParams, DagsterListJobsResponse } from '@/tools/dagster/types'
23
import type { ToolConfig } from '@/tools/types'
34

@@ -12,6 +13,10 @@ const LIST_JOBS_QUERY = `
1213
}
1314
}
1415
}
16+
... on Error {
17+
__typename
18+
message
19+
}
1520
}
1621
}
1722
`
@@ -53,38 +58,32 @@ export const listJobsTool: ToolConfig<DagsterBaseParams, DagsterListJobsResponse
5358
},
5459

5560
transformResponse: async (response: Response) => {
56-
const data = await response.json()
57-
58-
if (!response.ok) {
59-
throw new Error(data.errors?.[0]?.message || 'Dagster GraphQL request failed')
60-
}
61-
62-
if (data.errors?.length) {
63-
throw new Error(data.errors[0].message)
64-
}
61+
const data = await parseDagsterGraphqlResponse<{ repositoriesOrError?: unknown }>(response)
6562

66-
const result = data.data?.repositoriesOrError
63+
const result = data.data?.repositoriesOrError as
64+
| { nodes?: Array<{ name: string; jobs?: Array<{ name: string }> }>; message?: string }
65+
| undefined
6766
if (!result) throw new Error('Unexpected response from Dagster')
6867

69-
if (result.nodes != null) {
70-
const jobs: Array<{ name: string; repositoryName: string }> = []
68+
if (!Array.isArray(result.nodes)) {
69+
throw new Error(dagsterUnionErrorMessage(result, 'List jobs failed'))
70+
}
7171

72-
for (const repo of result.nodes ?? []) {
73-
for (const job of repo.jobs ?? []) {
74-
jobs.push({
75-
name: job.name,
76-
repositoryName: repo.name,
77-
})
78-
}
79-
}
72+
const jobs: Array<{ name: string; repositoryName: string }> = []
8073

81-
return {
82-
success: true,
83-
output: { jobs },
74+
for (const repo of result.nodes) {
75+
for (const job of repo.jobs ?? []) {
76+
jobs.push({
77+
name: job.name,
78+
repositoryName: repo.name,
79+
})
8480
}
8581
}
8682

87-
throw new Error(result.message || 'List jobs failed')
83+
return {
84+
success: true,
85+
output: { jobs },
86+
}
8887
},
8988

9089
outputs: {

‎apps/sim/tools/dagster/list_runs.ts‎

Lines changed: 30 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,17 @@
1+
import { dagsterUnionErrorMessage, parseDagsterGraphqlResponse } from '@/tools/dagster/graphql'
12
import type { DagsterListRunsParams, DagsterListRunsResponse } from '@/tools/dagster/types'
23
import type { ToolConfig } from '@/tools/types'
34

5+
/** Shape of each run in the `runsOrError` → `Runs.results` GraphQL selection set. */
6+
interface DagsterListRunsGraphqlRow {
7+
runId: string
8+
jobName: string | null
9+
status: string
10+
tags: Array<{ key: string; value: string }> | null
11+
startTime: number | null
12+
endTime: number | null
13+
}
14+
415
function buildListRunsQuery(hasFilter: boolean) {
516
return `
617
query ListRuns($limit: Int${hasFilter ? ', $filter: RunsFilter' : ''}) {
@@ -18,6 +29,10 @@ function buildListRunsQuery(hasFilter: boolean) {
1829
endTime
1930
}
2031
}
32+
... on Error {
33+
__typename
34+
message
35+
}
2136
}
2237
}
2338
`
@@ -93,36 +108,25 @@ export const listRunsTool: ToolConfig<DagsterListRunsParams, DagsterListRunsResp
93108
},
94109

95110
transformResponse: async (response: Response) => {
96-
const data = await response.json()
111+
const data = await parseDagsterGraphqlResponse<{ runsOrError?: unknown }>(response)
97112

98-
if (!response.ok) {
99-
throw new Error(data.errors?.[0]?.message || 'Dagster GraphQL request failed')
100-
}
113+
const result = data.data?.runsOrError as
114+
| { results?: DagsterListRunsGraphqlRow[]; message?: string }
115+
| undefined
116+
if (!result) throw new Error('Unexpected response from Dagster')
101117

102-
if (data.errors?.length) {
103-
throw new Error(data.errors[0].message)
118+
if (!Array.isArray(result.results)) {
119+
throw new Error(dagsterUnionErrorMessage(result, 'Dagster returned an error listing runs'))
104120
}
105121

106-
const result = data.data?.runsOrError
107-
if (!result) throw new Error('Unexpected response from Dagster')
108-
109-
const runs = (result.results ?? []).map(
110-
(r: {
111-
runId: string
112-
jobName: string | null
113-
status: string
114-
tags: Array<{ key: string; value: string }> | null
115-
startTime: number | null
116-
endTime: number | null
117-
}) => ({
118-
runId: r.runId,
119-
jobName: r.jobName ?? null,
120-
status: r.status,
121-
tags: r.tags ?? null,
122-
startTime: r.startTime ?? null,
123-
endTime: r.endTime ?? null,
124-
})
125-
)
122+
const runs = result.results.map((r: DagsterListRunsGraphqlRow) => ({
123+
runId: r.runId,
124+
jobName: r.jobName ?? null,
125+
status: r.status,
126+
tags: r.tags ?? null,
127+
startTime: r.startTime ?? null,
128+
endTime: r.endTime ?? null,
129+
}))
126130

127131
return {
128132
success: true,

0 commit comments

Comments
 (0)