Skip to content

Commit 4cbff46

Browse files
dmeranddmerand
committed
Expose structured app validation issues in JSON output
1 parent 46515e4 commit 4cbff46

File tree

4 files changed

+659
-17
lines changed

4 files changed

+659
-17
lines changed

‎packages/app/src/cli/commands/app/config/validate.test.ts‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,18 @@ import {linkedAppContext} from '../../../services/app-context.js'
33
import {validateApp} from '../../../services/validate.js'
44
import {testAppLinked} from '../../../models/app/app.test-data.js'
55
import {describe, expect, test, vi} from 'vitest'
6+
import {AbortError} from '@shopify/cli-kit/node/error'
7+
import {outputResult} from '@shopify/cli-kit/node/output'
68

79
vi.mock('../../../services/app-context.js')
810
vi.mock('../../../services/validate.js')
11+
vi.mock('@shopify/cli-kit/node/output', async (importOriginal) => {
12+
const actual = await importOriginal<typeof import('@shopify/cli-kit/node/output')>()
13+
return {
14+
...actual,
15+
outputResult: vi.fn(),
16+
}
17+
})
918

1019
describe('app config validate command', () => {
1120
test('calls validateApp with json: false by default', async () => {
@@ -46,4 +55,163 @@ describe('app config validate command', () => {
4655
// Then
4756
expect(validateApp).toHaveBeenCalledWith(app, {json: true})
4857
})
58+
59+
test('outputs json issues when app loading aborts before validateApp runs', async () => {
60+
// Given
61+
vi.mocked(linkedAppContext).mockRejectedValue(
62+
new AbortError('Validation errors in /tmp/shopify.app.toml:\n\n• [name]: String is required'),
63+
)
64+
65+
// When / Then
66+
await Validate.run(['--json', '--path=/tmp/app'], import.meta.url).catch(() => {})
67+
expect(outputResult).toHaveBeenCalledWith(
68+
JSON.stringify(
69+
{
70+
valid: false,
71+
issues: [
72+
{
73+
filePath: '/tmp/shopify.app.toml',
74+
path: [],
75+
pathString: 'name',
76+
message: 'String is required',
77+
},
78+
],
79+
},
80+
null,
81+
2,
82+
),
83+
)
84+
expect(validateApp).not.toHaveBeenCalled()
85+
})
86+
87+
test('outputs json issues when app loading aborts with ansi-colored structured text', async () => {
88+
// Given
89+
vi.mocked(linkedAppContext).mockRejectedValue(
90+
new AbortError(
91+
'\u001b[1m\u001b[91mValidation errors\u001b[39m\u001b[22m in /tmp/shopify.app.toml:\n\n• [name]: String is required',
92+
),
93+
)
94+
95+
// When / Then
96+
await Validate.run(['--json', '--path=/tmp/app'], import.meta.url).catch(() => {})
97+
expect(outputResult).toHaveBeenCalledWith(
98+
JSON.stringify(
99+
{
100+
valid: false,
101+
issues: [
102+
{
103+
filePath: '/tmp/shopify.app.toml',
104+
path: [],
105+
pathString: 'name',
106+
message: 'String is required',
107+
},
108+
],
109+
},
110+
null,
111+
2,
112+
),
113+
)
114+
expect(validateApp).not.toHaveBeenCalled()
115+
})
116+
117+
test('preserves a root json issue when contextual text precedes structured validation errors', async () => {
118+
// Given
119+
vi.mocked(linkedAppContext).mockRejectedValue(
120+
new AbortError(
121+
'Could not infer extension handle\n\nValidation errors in /tmp/shopify.app.toml:\n\n• [name]: String is required',
122+
),
123+
)
124+
125+
// When / Then
126+
await Validate.run(['--json', '--path=/tmp/app'], import.meta.url).catch(() => {})
127+
expect(outputResult).toHaveBeenCalledWith(
128+
JSON.stringify(
129+
{
130+
valid: false,
131+
issues: [
132+
{
133+
filePath: '/tmp/shopify.app.toml',
134+
path: [],
135+
pathString: 'name',
136+
message: 'String is required',
137+
},
138+
{
139+
filePath: '/tmp/shopify.app.toml',
140+
path: [],
141+
pathString: 'root',
142+
message:
143+
'Could not infer extension handle\n\nValidation errors in /tmp/shopify.app.toml:\n\n• [name]: String is required',
144+
},
145+
],
146+
},
147+
null,
148+
2,
149+
),
150+
)
151+
expect(validateApp).not.toHaveBeenCalled()
152+
})
153+
154+
test('parses structured validation errors for windows-style paths', async () => {
155+
// Given
156+
vi.mocked(linkedAppContext).mockRejectedValue(
157+
new AbortError('Validation errors in C:\\tmp\\shopify.app.toml:\n\n• [name]: String is required'),
158+
)
159+
160+
// When / Then
161+
await Validate.run(['--json', '--path=/tmp/app'], import.meta.url).catch(() => {})
162+
expect(outputResult).toHaveBeenCalledWith(
163+
JSON.stringify(
164+
{
165+
valid: false,
166+
issues: [
167+
{
168+
filePath: 'C:\\tmp\\shopify.app.toml',
169+
path: [],
170+
pathString: 'name',
171+
message: 'String is required',
172+
},
173+
],
174+
},
175+
null,
176+
2,
177+
),
178+
)
179+
expect(validateApp).not.toHaveBeenCalled()
180+
})
181+
182+
test('outputs a root json issue when app loading aborts with a non-structured message', async () => {
183+
// Given
184+
vi.mocked(linkedAppContext).mockRejectedValue(new AbortError("Couldn't find an app toml file at /tmp/app"))
185+
186+
// When / Then
187+
await Validate.run(['--json', '--path=/tmp/app'], import.meta.url).catch(() => {})
188+
expect(outputResult).toHaveBeenCalledWith(
189+
JSON.stringify(
190+
{
191+
valid: false,
192+
issues: [
193+
{
194+
filePath: '/tmp/app',
195+
path: [],
196+
pathString: 'root',
197+
message: "Couldn't find an app toml file at /tmp/app",
198+
},
199+
],
200+
},
201+
null,
202+
2,
203+
),
204+
)
205+
expect(validateApp).not.toHaveBeenCalled()
206+
})
207+
208+
test('rethrows unrelated abort errors in json mode without converting them to validation json', async () => {
209+
// Given
210+
vi.mocked(linkedAppContext).mockRejectedValue(new AbortError('Could not find store for domain shop.example.com'))
211+
212+
// When / Then
213+
await expect(Validate.run(['--json', '--path=/tmp/app'], import.meta.url)).rejects.toThrow()
214+
expect(outputResult).not.toHaveBeenCalled()
215+
expect(validateApp).not.toHaveBeenCalled()
216+
})
49217
})

‎packages/app/src/cli/commands/app/config/validate.ts‎

Lines changed: 97 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,76 @@ import {validateApp} from '../../../services/validate.js'
33
import AppLinkedCommand, {AppLinkedCommandOutput} from '../../../utilities/app-linked-command.js'
44
import {linkedAppContext} from '../../../services/app-context.js'
55
import {globalFlags, jsonFlag} from '@shopify/cli-kit/node/cli'
6+
import {AbortError, AbortSilentError} from '@shopify/cli-kit/node/error'
7+
import {outputResult, unstyled} from '@shopify/cli-kit/node/output'
8+
9+
import type {AppValidationIssue} from '../../../models/app/error-parsing.js'
10+
11+
function toRootIssue(filePath: string, message: string): AppValidationIssue {
12+
return {
13+
filePath,
14+
path: [],
15+
pathString: 'root',
16+
message,
17+
}
18+
}
19+
20+
function hasMeaningfulPrefix(prefix: string): boolean {
21+
const normalizedPrefix = prefix.trim()
22+
return normalizedPrefix !== '' && normalizedPrefix !== 'App configuration is not valid'
23+
}
24+
25+
function isJsonValidationAbort(error: AbortError): boolean {
26+
const message = unstyled(error.message).trim()
27+
28+
return (
29+
message.includes('Validation errors in ') ||
30+
message.startsWith("Couldn't find an app toml file at") ||
31+
message.startsWith("Couldn't find directory ") ||
32+
/^Couldn't find .* in .+\.?$/.test(message)
33+
)
34+
}
35+
36+
function toJsonIssuesFromAbortError(error: AbortError, fallbackFilePath: string): AppValidationIssue[] {
37+
const message = unstyled(error.message).trim()
38+
const marker = 'Validation errors in '
39+
const markerIndex = message.indexOf(marker)
40+
41+
if (markerIndex === -1) {
42+
return [toRootIssue(fallbackFilePath, message)]
43+
}
44+
45+
const bodyStartIndex = message.indexOf('\n\n', markerIndex)
46+
if (bodyStartIndex === -1) {
47+
return [toRootIssue(fallbackFilePath, message)]
48+
}
49+
50+
const filePathLine = message.slice(markerIndex + marker.length, bodyStartIndex)
51+
if (!filePathLine.endsWith(':')) {
52+
return [toRootIssue(fallbackFilePath, message)]
53+
}
54+
55+
const filePath = filePathLine.slice(0, -1)
56+
const body = message.slice(bodyStartIndex + 2)
57+
const issues = Array.from(body.matchAll(/^• \[([^\]]+)\]: (.+)$/gm)).map((captures) => {
58+
const pathString = captures[1]!
59+
const issueMessage = captures[2]!
60+
61+
return {
62+
filePath,
63+
// `pathString` is rendered display text, not a lossless encoding of the
64+
// original structured path, so the early-abort fallback avoids
65+
// reconstructing `path` from it.
66+
path: [],
67+
pathString,
68+
message: issueMessage,
69+
}
70+
})
71+
72+
if (issues.length === 0) return [toRootIssue(filePath, message)]
73+
if (hasMeaningfulPrefix(message.slice(0, markerIndex))) return [...issues, toRootIssue(filePath, message)]
74+
return issues
75+
}
676

777
export default class Validate extends AppLinkedCommand {
878
static summary = 'Validate your app configuration and extensions.'
@@ -20,16 +90,34 @@ export default class Validate extends AppLinkedCommand {
2090
public async run(): Promise<AppLinkedCommandOutput> {
2191
const {flags} = await this.parse(Validate)
2292

23-
const {app} = await linkedAppContext({
24-
directory: flags.path,
25-
clientId: flags['client-id'],
26-
forceRelink: flags.reset,
27-
userProvidedConfigName: flags.config,
28-
unsafeReportMode: true,
29-
})
93+
try {
94+
const {app} = await linkedAppContext({
95+
directory: flags.path,
96+
clientId: flags['client-id'],
97+
forceRelink: flags.reset,
98+
userProvidedConfigName: flags.config,
99+
unsafeReportMode: true,
100+
})
101+
102+
await validateApp(app, {json: flags.json})
30103

31-
await validateApp(app, {json: flags.json})
104+
return {app}
105+
} catch (error) {
106+
if (flags.json && error instanceof AbortError && isJsonValidationAbort(error)) {
107+
outputResult(
108+
JSON.stringify(
109+
{
110+
valid: false,
111+
issues: toJsonIssuesFromAbortError(error, flags.path),
112+
},
113+
null,
114+
2,
115+
),
116+
)
117+
throw new AbortSilentError()
118+
}
32119

33-
return {app}
120+
throw error
121+
}
34122
}
35123
}

0 commit comments

Comments
 (0)