Skip to content

Commit edb96a0

Browse files
dmeranddmerand
committed
Introduce AppConfigurationAbortError for app validate json
1 parent 4cbff46 commit edb96a0

File tree

8 files changed

+254
-161
lines changed

8 files changed

+254
-161
lines changed

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

Lines changed: 56 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ 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'
67
import {AbortError} from '@shopify/cli-kit/node/error'
78
import {outputResult} from '@shopify/cli-kit/node/output'
@@ -56,52 +57,48 @@ describe('app config validate command', () => {
5657
expect(validateApp).toHaveBeenCalledWith(app, {json: true})
5758
})
5859

59-
test('outputs json issues when app loading aborts before validateApp runs', async () => {
60+
test('rethrows AppConfigurationAbortError in non-json mode without emitting json', async () => {
6061
// Given
6162
vi.mocked(linkedAppContext).mockRejectedValue(
62-
new AbortError('Validation errors in /tmp/shopify.app.toml:\n\n• [name]: String is required'),
63+
new AppConfigurationAbortError('Validation errors in /tmp/shopify.app.toml', '/tmp/shopify.app.toml'),
6364
)
6465

6566
// 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-
)
67+
await expect(Validate.run(['--path=/tmp/app'], import.meta.url)).rejects.toThrow()
68+
expect(outputResult).not.toHaveBeenCalled()
8469
expect(validateApp).not.toHaveBeenCalled()
8570
})
8671

87-
test('outputs json issues when app loading aborts with ansi-colored structured text', async () => {
72+
test('outputs structured configuration issues from app loading before validateApp runs', async () => {
8873
// Given
8974
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',
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+
],
9286
),
9387
)
9488

9589
// When / Then
96-
await Validate.run(['--json', '--path=/tmp/app'], import.meta.url).catch(() => {})
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)
9794
expect(outputResult).toHaveBeenCalledWith(
9895
JSON.stringify(
9996
{
10097
valid: false,
10198
issues: [
10299
{
103100
filePath: '/tmp/shopify.app.toml',
104-
path: [],
101+
path: ['name'],
105102
pathString: 'name',
106103
message: 'String is required',
107104
},
@@ -114,33 +111,27 @@ describe('app config validate command', () => {
114111
expect(validateApp).not.toHaveBeenCalled()
115112
})
116113

117-
test('preserves a root json issue when contextual text precedes structured validation errors', async () => {
114+
test('outputs a root json issue when app loading fails without structured issues', async () => {
118115
// Given
119116
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-
),
117+
new AppConfigurationAbortError("Couldn't find an app toml file at /tmp/app", '/tmp/app'),
123118
)
124119

125120
// When / Then
126-
await Validate.run(['--json', '--path=/tmp/app'], import.meta.url).catch(() => {})
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)
127125
expect(outputResult).toHaveBeenCalledWith(
128126
JSON.stringify(
129127
{
130128
valid: false,
131129
issues: [
132130
{
133-
filePath: '/tmp/shopify.app.toml',
134-
path: [],
135-
pathString: 'name',
136-
message: 'String is required',
137-
},
138-
{
139-
filePath: '/tmp/shopify.app.toml',
131+
filePath: '/tmp/app',
140132
path: [],
141133
pathString: 'root',
142-
message:
143-
'Could not infer extension handle\n\nValidation errors in /tmp/shopify.app.toml:\n\n• [name]: String is required',
134+
message: "Couldn't find an app toml file at /tmp/app",
144135
},
145136
],
146137
},
@@ -151,22 +142,36 @@ describe('app config validate command', () => {
151142
expect(validateApp).not.toHaveBeenCalled()
152143
})
153144

154-
test('parses structured validation errors for windows-style paths', async () => {
145+
test('outputs json when validateApp throws a structured configuration abort', async () => {
155146
// Given
156-
vi.mocked(linkedAppContext).mockRejectedValue(
157-
new AbortError('Validation errors in C:\\tmp\\shopify.app.toml:\n\n• [name]: String is required'),
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+
),
158162
)
159163

160164
// When / Then
161-
await Validate.run(['--json', '--path=/tmp/app'], import.meta.url).catch(() => {})
165+
await expect(Validate.run(['--json'], import.meta.url)).rejects.toThrow('process.exit unexpectedly called with "1"')
166+
expect(outputResult).toHaveBeenCalledTimes(1)
162167
expect(outputResult).toHaveBeenCalledWith(
163168
JSON.stringify(
164169
{
165170
valid: false,
166171
issues: [
167172
{
168-
filePath: 'C:\\tmp\\shopify.app.toml',
169-
path: [],
173+
filePath: '/tmp/shopify.app.toml',
174+
path: ['name'],
170175
pathString: 'name',
171176
message: 'String is required',
172177
},
@@ -176,33 +181,17 @@ describe('app config validate command', () => {
176181
2,
177182
),
178183
)
179-
expect(validateApp).not.toHaveBeenCalled()
180184
})
181185

182-
test('outputs a root json issue when app loading aborts with a non-structured message', async () => {
186+
test('rethrows non-configuration errors from validateApp in json mode without converting them to validation json', async () => {
183187
// Given
184-
vi.mocked(linkedAppContext).mockRejectedValue(new AbortError("Couldn't find an app toml file at /tmp/app"))
188+
const app = testAppLinked()
189+
vi.mocked(linkedAppContext).mockResolvedValue({app} as Awaited<ReturnType<typeof linkedAppContext>>)
190+
vi.mocked(validateApp).mockRejectedValue(new AbortError('network problem'))
185191

186192
// 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()
193+
await expect(Validate.run(['--json'], import.meta.url)).rejects.toThrow()
194+
expect(outputResult).not.toHaveBeenCalled()
206195
})
207196

208197
test('rethrows unrelated abort errors in json mode without converting them to validation json', async () => {

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

Lines changed: 6 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -2,77 +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'
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-
}
8+
import {AbortSilentError} from '@shopify/cli-kit/node/error'
9+
import {outputResult} from '@shopify/cli-kit/node/output'
7610

7711
export default class Validate extends AppLinkedCommand {
7812
static summary = 'Validate your app configuration and extensions.'
@@ -103,17 +37,8 @@ export default class Validate extends AppLinkedCommand {
10337

10438
return {app}
10539
} 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-
)
40+
if (flags.json && error instanceof AppConfigurationAbortError) {
41+
outputResult(stringifyAppValidationResult(invalidAppValidationResult(error.issues)))
11742
throw new AbortSilentError()
11843
}
11944

‎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 = [

0 commit comments

Comments
 (0)