Skip to content

Commit a7602f8

Browse files
committed
Preserve structured app validation issues internally
1 parent 5eca6a9 commit a7602f8

File tree

5 files changed

+442
-24
lines changed

5 files changed

+442
-24
lines changed

packages/app/src/cli/models/app/error-parsing.test.ts

Lines changed: 148 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import {parseHumanReadableError} from './error-parsing.js'
1+
import {parseHumanReadableError, parseStructuredErrors} from './error-parsing.js'
22
import {describe, expect, test} from 'vitest'
33

44
describe('parseHumanReadableError', () => {
@@ -235,3 +235,150 @@ describe('parseHumanReadableError', () => {
235235
expect(result).not.toContain('Union validation failed')
236236
})
237237
})
238+
239+
describe('parseStructuredErrors', () => {
240+
test('preserves regular issues with file path and path string', () => {
241+
const issues = [
242+
{
243+
path: ['name'],
244+
message: 'Required field is missing',
245+
code: 'invalid_type',
246+
},
247+
{
248+
path: ['version'],
249+
message: 'Must be a valid semver string',
250+
code: 'custom',
251+
},
252+
]
253+
254+
const result = parseStructuredErrors(issues, '/tmp/shopify.app.toml')
255+
256+
expect(result).toEqual([
257+
{
258+
filePath: '/tmp/shopify.app.toml',
259+
path: ['name'],
260+
pathString: 'name',
261+
message: 'Required field is missing',
262+
code: 'invalid_type',
263+
},
264+
{
265+
filePath: '/tmp/shopify.app.toml',
266+
path: ['version'],
267+
pathString: 'version',
268+
message: 'Must be a valid semver string',
269+
code: 'custom',
270+
},
271+
])
272+
})
273+
274+
test('uses the best matching union variant for structured issues', () => {
275+
const issues = [
276+
{
277+
code: 'invalid_union',
278+
unionErrors: [
279+
{
280+
issues: [
281+
{
282+
code: 'invalid_type',
283+
path: ['web', 'roles'],
284+
message: 'Expected array, received number',
285+
},
286+
{
287+
code: 'invalid_type',
288+
path: ['web', 'commands', 'build'],
289+
message: 'Required',
290+
},
291+
],
292+
name: 'ZodError',
293+
},
294+
{
295+
issues: [
296+
{
297+
code: 'invalid_literal',
298+
path: ['web', 'type'],
299+
message: "Invalid literal value, expected 'frontend'",
300+
},
301+
],
302+
name: 'ZodError',
303+
},
304+
],
305+
path: ['web'],
306+
message: 'Invalid input',
307+
},
308+
]
309+
310+
const result = parseStructuredErrors(issues, '/tmp/shopify.web.toml')
311+
312+
expect(result).toEqual([
313+
{
314+
filePath: '/tmp/shopify.web.toml',
315+
path: ['web', 'roles'],
316+
pathString: 'web.roles',
317+
message: 'Expected array, received number',
318+
code: 'invalid_type',
319+
},
320+
{
321+
filePath: '/tmp/shopify.web.toml',
322+
path: ['web', 'commands', 'build'],
323+
pathString: 'web.commands.build',
324+
message: 'Required',
325+
code: 'invalid_type',
326+
},
327+
])
328+
})
329+
330+
test('falls back to recovered nested union issues before returning a synthetic root issue', () => {
331+
const unionIssues = Array.from({length: 51}, (_, index) => ({
332+
code: 'custom',
333+
path: ['variants', index],
334+
message: `Invalid variant ${index + 1}`,
335+
}))
336+
337+
const issues = [
338+
{
339+
code: 'invalid_union',
340+
unionErrors: [{issues: unionIssues, name: 'ZodError'}],
341+
path: ['root'],
342+
message: 'Invalid input',
343+
},
344+
]
345+
346+
const result = parseStructuredErrors(issues, '/tmp/shopify.app.toml')
347+
348+
expect(result).toEqual(
349+
unionIssues.map((issue, index) => ({
350+
filePath: '/tmp/shopify.app.toml',
351+
path: ['variants', index],
352+
pathString: `variants.${index}`,
353+
message: issue.message,
354+
code: 'custom',
355+
})),
356+
)
357+
})
358+
359+
test('falls back to a synthetic root issue when union variants expose no nested issues', () => {
360+
const issues = [
361+
{
362+
code: 'invalid_union',
363+
unionErrors: [
364+
{issues: [], name: 'ZodError'},
365+
{issues: [], name: 'ZodError'},
366+
],
367+
path: ['root'],
368+
message: 'Invalid input',
369+
},
370+
]
371+
372+
const result = parseStructuredErrors(issues, '/tmp/shopify.app.toml')
373+
374+
expect(result).toEqual([
375+
{
376+
filePath: '/tmp/shopify.app.toml',
377+
path: ['root'],
378+
pathString: 'root',
379+
message: "Configuration doesn't match any expected format",
380+
code: 'invalid_union',
381+
},
382+
])
383+
})
384+
})

packages/app/src/cli/models/app/error-parsing.ts

Lines changed: 78 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
1+
import type {OutputMessage} from '@shopify/cli-kit/node/output'
2+
3+
interface UnionErrorIssue {
4+
path?: (string | number)[]
5+
message: string
6+
code?: string
7+
}
8+
19
interface UnionError {
2-
issues?: {path?: (string | number)[]; message: string}[]
10+
issues?: UnionErrorIssue[]
311
name: string
412
}
513

@@ -10,6 +18,20 @@ interface ExtendedZodIssue {
1018
unionErrors?: UnionError[]
1119
}
1220

21+
export interface AppValidationIssue {
22+
filePath: string
23+
path: (string | number)[]
24+
pathString: string
25+
message: string
26+
code?: string
27+
}
28+
29+
export interface AppValidationFileIssues {
30+
filePath: string
31+
message: OutputMessage
32+
issues: AppValidationIssue[]
33+
}
34+
1335
/**
1436
* Finds the best matching variant from a union error by scoring each variant
1537
* based on how close it is to the user's likely intent.
@@ -56,13 +78,66 @@ function findBestMatchingVariant(unionErrors: UnionError[]): UnionError | null {
5678
return bestVariant
5779
}
5880

81+
function getIssuePath(path?: (string | number)[]) {
82+
return path ?? []
83+
}
84+
85+
function getIssuePathString(path?: (string | number)[]) {
86+
const resolvedPath = getIssuePath(path)
87+
return resolvedPath.length > 0 ? resolvedPath.map(String).join('.') : 'root'
88+
}
89+
5990
/**
6091
* Formats an issue into a human-readable error line
6192
*/
6293
function formatErrorLine(issue: {path?: (string | number)[]; message?: string}, indent = '') {
63-
const path = issue.path && issue.path.length > 0 ? issue.path.map(String).join('.') : 'root'
94+
const pathString = getIssuePathString(issue.path)
6495
const message = issue.message ?? 'Unknown error'
65-
return `${indent}• [${path}]: ${message}\n`
96+
return `${indent}• [${pathString}]: ${message}\n`
97+
}
98+
99+
function toStructuredIssue(filePath: string, issue: Pick<ExtendedZodIssue, 'path' | 'message' | 'code'>) {
100+
return {
101+
filePath,
102+
path: getIssuePath(issue.path),
103+
pathString: getIssuePathString(issue.path),
104+
message: issue.message ?? 'Unknown error',
105+
code: issue.code,
106+
}
107+
}
108+
109+
export function parseStructuredErrors(issues: ExtendedZodIssue[], filePath: string): AppValidationIssue[] {
110+
return issues.flatMap((issue) => {
111+
if (issue.code === 'invalid_union' && issue.unionErrors) {
112+
// Intentionally mirror the current human-readable union selection heuristic
113+
// so structured/internal issues stay aligned with existing CLI behavior.
114+
// If we change this heuristic later, text and structured output should move together.
115+
const bestVariant = findBestMatchingVariant(issue.unionErrors)
116+
117+
if (bestVariant?.issues?.length) {
118+
return bestVariant.issues.map((nestedIssue) => toStructuredIssue(filePath, nestedIssue))
119+
}
120+
121+
const fallbackIssues = issue.unionErrors.flatMap((unionError) => unionError.issues ?? [])
122+
if (fallbackIssues.length > 0) {
123+
// Preserve any concrete nested issues we were able to recover before
124+
// falling back to a synthetic root issue. This structured path is still
125+
// internal, and retaining leaf issues is more actionable than erasing
126+
// them behind a generic union failure.
127+
return fallbackIssues.map((nestedIssue) => toStructuredIssue(filePath, nestedIssue))
128+
}
129+
130+
return [
131+
toStructuredIssue(filePath, {
132+
path: issue.path,
133+
message: "Configuration doesn't match any expected format",
134+
code: issue.code,
135+
}),
136+
]
137+
}
138+
139+
return [toStructuredIssue(filePath, issue)]
140+
})
66141
}
67142

68143
export function parseHumanReadableError(issues: ExtendedZodIssue[]) {

0 commit comments

Comments
 (0)