Skip to content

Commit f321549

Browse files
dmeranddmerand
committed
Introduce AppConfigurationAbortError for app validate json
1 parent 123cec3 commit f321549

8 files changed

Lines changed: 364 additions & 20 deletions

File tree

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

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,20 @@ import Validate from './validate.js'
22
import {linkedAppContext} from '../../../services/app-context.js'
33
import {validateApp} from '../../../services/validate.js'
44
import {testAppLinked} from '../../../models/app/app.test-data.js'
5+
import {AppConfigurationAbortError} from '../../../models/app/error-parsing.js'
56
import {describe, expect, test, vi} from 'vitest'
7+
import {AbortError} from '@shopify/cli-kit/node/error'
8+
import {outputResult} from '@shopify/cli-kit/node/output'
69

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

1020
describe('app config validate command', () => {
1121
test('calls validateApp with json: false by default', async () => {
@@ -46,4 +56,151 @@ describe('app config validate command', () => {
4656
// Then
4757
expect(validateApp).toHaveBeenCalledWith(app, {json: true})
4858
})
59+
60+
test('rethrows AppConfigurationAbortError in non-json mode without emitting json', async () => {
61+
// Given
62+
vi.mocked(linkedAppContext).mockRejectedValue(
63+
new AppConfigurationAbortError('Validation errors in /tmp/shopify.app.toml', '/tmp/shopify.app.toml'),
64+
)
65+
66+
// When / Then
67+
await expect(Validate.run(['--path=/tmp/app'], import.meta.url)).rejects.toThrow()
68+
expect(outputResult).not.toHaveBeenCalled()
69+
expect(validateApp).not.toHaveBeenCalled()
70+
})
71+
72+
test('outputs structured configuration issues from app loading before validateApp runs', async () => {
73+
// Given
74+
vi.mocked(linkedAppContext).mockRejectedValue(
75+
new AppConfigurationAbortError(
76+
'Validation errors in /tmp/shopify.app.toml:\n\n• [name]: String is required',
77+
'/tmp/shopify.app.toml',
78+
[
79+
{
80+
filePath: '/tmp/shopify.app.toml',
81+
path: ['name'],
82+
pathString: 'name',
83+
message: 'String is required',
84+
},
85+
],
86+
),
87+
)
88+
89+
// When / Then
90+
await expect(Validate.run(['--json', '--path=/tmp/app'], import.meta.url)).rejects.toThrow(
91+
'process.exit unexpectedly called with "1"',
92+
)
93+
expect(outputResult).toHaveBeenCalledTimes(1)
94+
expect(outputResult).toHaveBeenCalledWith(
95+
JSON.stringify(
96+
{
97+
valid: false,
98+
issues: [
99+
{
100+
filePath: '/tmp/shopify.app.toml',
101+
path: ['name'],
102+
pathString: 'name',
103+
message: 'String is required',
104+
},
105+
],
106+
},
107+
null,
108+
2,
109+
),
110+
)
111+
expect(validateApp).not.toHaveBeenCalled()
112+
})
113+
114+
test('outputs a root json issue when app loading fails without structured issues', async () => {
115+
// Given
116+
vi.mocked(linkedAppContext).mockRejectedValue(
117+
new AppConfigurationAbortError("Couldn't find an app toml file at /tmp/app", '/tmp/app'),
118+
)
119+
120+
// When / Then
121+
await expect(Validate.run(['--json', '--path=/tmp/app'], import.meta.url)).rejects.toThrow(
122+
'process.exit unexpectedly called with "1"',
123+
)
124+
expect(outputResult).toHaveBeenCalledTimes(1)
125+
expect(outputResult).toHaveBeenCalledWith(
126+
JSON.stringify(
127+
{
128+
valid: false,
129+
issues: [
130+
{
131+
filePath: '/tmp/app',
132+
path: [],
133+
pathString: 'root',
134+
message: "Couldn't find an app toml file at /tmp/app",
135+
},
136+
],
137+
},
138+
null,
139+
2,
140+
),
141+
)
142+
expect(validateApp).not.toHaveBeenCalled()
143+
})
144+
145+
test('outputs json when validateApp throws a structured configuration abort', async () => {
146+
// Given
147+
const app = testAppLinked()
148+
vi.mocked(linkedAppContext).mockResolvedValue({app} as Awaited<ReturnType<typeof linkedAppContext>>)
149+
vi.mocked(validateApp).mockRejectedValue(
150+
new AppConfigurationAbortError(
151+
'Validation errors in /tmp/shopify.app.toml:\n\n• [name]: String is required',
152+
'/tmp/shopify.app.toml',
153+
[
154+
{
155+
filePath: '/tmp/shopify.app.toml',
156+
path: ['name'],
157+
pathString: 'name',
158+
message: 'String is required',
159+
},
160+
],
161+
),
162+
)
163+
164+
// When / Then
165+
await expect(Validate.run(['--json'], import.meta.url)).rejects.toThrow('process.exit unexpectedly called with "1"')
166+
expect(outputResult).toHaveBeenCalledTimes(1)
167+
expect(outputResult).toHaveBeenCalledWith(
168+
JSON.stringify(
169+
{
170+
valid: false,
171+
issues: [
172+
{
173+
filePath: '/tmp/shopify.app.toml',
174+
path: ['name'],
175+
pathString: 'name',
176+
message: 'String is required',
177+
},
178+
],
179+
},
180+
null,
181+
2,
182+
),
183+
)
184+
})
185+
186+
test('rethrows non-configuration errors from validateApp in json mode without converting them to validation json', async () => {
187+
// Given
188+
const app = testAppLinked()
189+
vi.mocked(linkedAppContext).mockResolvedValue({app} as Awaited<ReturnType<typeof linkedAppContext>>)
190+
vi.mocked(validateApp).mockRejectedValue(new AbortError('network problem'))
191+
192+
// When / Then
193+
await expect(Validate.run(['--json'], import.meta.url)).rejects.toThrow()
194+
expect(outputResult).not.toHaveBeenCalled()
195+
})
196+
197+
test('rethrows unrelated abort errors in json mode without converting them to validation json', async () => {
198+
// Given
199+
vi.mocked(linkedAppContext).mockRejectedValue(new AbortError('Could not find store for domain shop.example.com'))
200+
201+
// When / Then
202+
await expect(Validate.run(['--json', '--path=/tmp/app'], import.meta.url)).rejects.toThrow()
203+
expect(outputResult).not.toHaveBeenCalled()
204+
expect(validateApp).not.toHaveBeenCalled()
205+
})
49206
})

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

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,11 @@ import {appFlags} from '../../../flags.js'
22
import {validateApp} from '../../../services/validate.js'
33
import AppLinkedCommand, {AppLinkedCommandOutput} from '../../../utilities/app-linked-command.js'
44
import {linkedAppContext} from '../../../services/app-context.js'
5+
import {AppConfigurationAbortError} from '../../../models/app/error-parsing.js'
6+
import {invalidAppValidationResult, stringifyAppValidationResult} from '../../../services/validation-result.js'
57
import {globalFlags, jsonFlag} from '@shopify/cli-kit/node/cli'
8+
import {AbortSilentError} from '@shopify/cli-kit/node/error'
9+
import {outputResult} from '@shopify/cli-kit/node/output'
610

711
export default class Validate extends AppLinkedCommand {
812
static summary = 'Validate your app configuration and extensions.'
@@ -20,16 +24,25 @@ export default class Validate extends AppLinkedCommand {
2024
public async run(): Promise<AppLinkedCommandOutput> {
2125
const {flags} = await this.parse(Validate)
2226

23-
const {app} = await linkedAppContext({
24-
directory: flags.path,
25-
clientId: flags['client-id'],
26-
forceRelink: flags.reset,
27-
userProvidedConfigName: flags.config,
28-
unsafeTolerateErrors: true,
29-
})
30-
31-
await validateApp(app, {json: flags.json})
32-
33-
return {app}
27+
try {
28+
const {app} = await linkedAppContext({
29+
directory: flags.path,
30+
clientId: flags['client-id'],
31+
forceRelink: flags.reset,
32+
userProvidedConfigName: flags.config,
33+
unsafeTolerateErrors: true,
34+
})
35+
36+
await validateApp(app, {json: flags.json})
37+
38+
return {app}
39+
} catch (error) {
40+
if (flags.json && error instanceof AppConfigurationAbortError) {
41+
outputResult(stringifyAppValidationResult(invalidAppValidationResult(error.issues)))
42+
throw new AbortSilentError()
43+
}
44+
45+
throw error
46+
}
3447
}
3548
}

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

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

44
describe('parseHumanReadableError', () => {
@@ -236,6 +236,47 @@ describe('parseHumanReadableError', () => {
236236
})
237237
})
238238

239+
describe('AppConfigurationAbortError', () => {
240+
test('preserves structured issues when they are provided', () => {
241+
const error = new AppConfigurationAbortError(
242+
'Validation errors in /tmp/shopify.app.toml',
243+
'/tmp/shopify.app.toml',
244+
[
245+
{
246+
filePath: '/tmp/shopify.app.toml',
247+
path: ['name'],
248+
pathString: 'name',
249+
message: 'Required',
250+
code: 'invalid_type',
251+
},
252+
],
253+
)
254+
255+
expect(error.issues).toEqual([
256+
{
257+
filePath: '/tmp/shopify.app.toml',
258+
path: ['name'],
259+
pathString: 'name',
260+
message: 'Required',
261+
code: 'invalid_type',
262+
},
263+
])
264+
})
265+
266+
test('synthesizes a root issue when structured issues are unavailable', () => {
267+
const error = new AppConfigurationAbortError("Couldn't find an app toml file at /tmp/app", '/tmp/app')
268+
269+
expect(error.issues).toEqual([
270+
{
271+
filePath: '/tmp/app',
272+
path: [],
273+
pathString: 'root',
274+
message: "Couldn't find an app toml file at /tmp/app",
275+
},
276+
])
277+
})
278+
})
279+
239280
describe('parseStructuredErrors', () => {
240281
test('preserves regular issues with file path and path string', () => {
241282
const issues = [

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
import {AbortError} from '@shopify/cli-kit/node/error'
2+
import {stringifyMessage} from '@shopify/cli-kit/node/output'
3+
14
import type {OutputMessage} from '@shopify/cli-kit/node/output'
25

36
interface UnionErrorIssue {
@@ -32,6 +35,42 @@ export interface AppValidationFileIssues {
3235
issues: AppValidationIssue[]
3336
}
3437

38+
export function toRootValidationIssue(filePath: string, message: string): AppValidationIssue {
39+
return {
40+
filePath,
41+
path: [],
42+
pathString: 'root',
43+
message,
44+
}
45+
}
46+
47+
function getValidationIssues(
48+
filePath: string,
49+
message: OutputMessage,
50+
issues: AppValidationIssue[],
51+
): AppValidationIssue[] {
52+
if (issues.length > 0) return issues
53+
54+
return [toRootValidationIssue(filePath, stringifyMessage(message).trim())]
55+
}
56+
57+
/**
58+
* Structured failure for local app configuration discovery, parsing, and
59+
* validation.
60+
*
61+
* This error carries the machine-readable issues used by
62+
* `shopify app validate --json` when local configuration loading cannot reach
63+
* a normal validation result.
64+
*/
65+
export class AppConfigurationAbortError extends AbortError {
66+
issues: AppValidationIssue[]
67+
68+
constructor(message: OutputMessage, filePath: string, issues: AppValidationIssue[] = []) {
69+
super(message)
70+
this.issues = getValidationIssues(filePath, message, issues)
71+
}
72+
}
73+
3574
/**
3675
* Finds the best matching variant from a union error by scoring each variant
3776
* based on how close it is to the user's likely intent.

‎packages/app/src/cli/models/app/loader.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import {
1313
AppLinkedInterface,
1414
} from './app.js'
1515
import {
16+
AppConfigurationAbortError,
1617
AppValidationFileIssues,
1718
AppValidationIssue,
1819
parseHumanReadableError,
@@ -71,13 +72,13 @@ interface ReloadState {
7172
previousDevURLs?: ApplicationURLs
7273
}
7374

74-
export class ConfigurationError extends AbortError {
75+
export class ConfigurationError extends AppConfigurationAbortError {
7576
constructor(
7677
message: OutputMessage,
7778
public readonly configurationPath: string,
7879
public readonly issues: AppValidationIssue[] = [],
7980
) {
80-
super(message)
81+
super(message, configurationPath, issues)
8182
}
8283
}
8384

@@ -977,7 +978,10 @@ async function getConfigurationPath(appDirectory: string, configName: string | u
977978
if (await fileExists(configurationPath)) {
978979
return {configurationPath, configurationFileName}
979980
} else {
980-
throw new AbortError(outputContent`Couldn't find ${configurationFileName} in ${outputToken.path(appDirectory)}.`)
981+
throw new AppConfigurationAbortError(
982+
outputContent`Couldn't find ${configurationFileName} in ${outputToken.path(appDirectory)}.`,
983+
configurationPath,
984+
)
981985
}
982986
}
983987

‎packages/app/src/cli/services/validate.test.ts‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ describe('validateApp', () => {
3737
await validateApp(app, {json: true})
3838

3939
// Then
40-
expect(outputResult).toHaveBeenCalledWith(JSON.stringify({valid: true, errors: []}, null, 2))
40+
expect(outputResult).toHaveBeenCalledWith(JSON.stringify({valid: true, issues: []}, null, 2))
4141
expect(renderSuccess).not.toHaveBeenCalled()
4242
expect(renderError).not.toHaveBeenCalled()
4343
})
@@ -74,7 +74,20 @@ describe('validateApp', () => {
7474
JSON.stringify(
7575
{
7676
valid: false,
77-
errors: ['client_id is required', 'invalid type "unknown"'],
77+
issues: [
78+
{
79+
filePath: '/path/to/shopify.app.toml',
80+
path: [],
81+
pathString: 'root',
82+
message: 'client_id is required',
83+
},
84+
{
85+
filePath: '/path/to/extensions/my-ext/shopify.extension.toml',
86+
path: [],
87+
pathString: 'root',
88+
message: 'invalid type "unknown"',
89+
},
90+
],
7891
},
7992
null,
8093
2,

0 commit comments

Comments
 (0)