Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 95 additions & 0 deletions packages/app/src/cli/commands/app/config/validate.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ import {testAppLinked} from '../../../models/app/app.test-data.js'
import {Project} from '../../../models/project/project.js'
import {selectActiveConfig} from '../../../models/project/active-config.js'
import {errorsForConfig} from '../../../models/project/config-selection.js'
import metadata from '../../../metadata.js'
import {outputResult} from '@shopify/cli-kit/node/output'
import {AbortError} from '@shopify/cli-kit/node/error'
import {TomlFile} from '@shopify/cli-kit/node/toml/toml-file'
import {describe, expect, test, vi} from 'vitest'

Expand All @@ -14,12 +16,19 @@ vi.mock('../../../services/validate.js')
vi.mock('../../../models/project/project.js')
vi.mock('../../../models/project/active-config.js')
vi.mock('../../../models/project/config-selection.js')
vi.mock('../../../metadata.js', () => ({default: {addPublicMetadata: vi.fn()}}))
vi.mock('@shopify/cli-kit/node/output', async (importOriginal) => {
const actual = await importOriginal<typeof import('@shopify/cli-kit/node/output')>()
return {...actual, outputResult: vi.fn()}
})
vi.mock('@shopify/cli-kit/node/ui')

async function expectValidationMetadataCalls(...expectedMetadata: Record<string, unknown>[]) {
const metadataCalls = vi.mocked(metadata.addPublicMetadata).mock.calls.map(([getMetadata]) => getMetadata)
expect(metadataCalls).toHaveLength(expectedMetadata.length)
await expect(Promise.all(metadataCalls.map((getMetadata) => getMetadata()))).resolves.toEqual(expectedMetadata)
}

function mockHealthyProject() {
vi.mocked(Project.load).mockResolvedValue({errors: []} as unknown as Project)
vi.mocked(selectActiveConfig).mockResolvedValue({file: new TomlFile('shopify.app.toml', {})} as any)
Expand All @@ -36,6 +45,7 @@ describe('app config validate command', () => {
await Validate.run([], import.meta.url)

expect(validateApp).toHaveBeenCalledWith(app, {json: false})
await expectValidationMetadataCalls({cmd_app_validate_json: false})
})

test('calls validateApp with json: true when --json flag is passed', async () => {
Expand All @@ -47,6 +57,7 @@ describe('app config validate command', () => {
await Validate.run(['--json'], import.meta.url)

expect(validateApp).toHaveBeenCalledWith(app, {json: true})
await expectValidationMetadataCalls({cmd_app_validate_json: true})
})

test('calls validateApp with json: true when -j flag is passed', async () => {
Expand All @@ -58,6 +69,7 @@ describe('app config validate command', () => {
await Validate.run(['-j'], import.meta.url)

expect(validateApp).toHaveBeenCalledWith(app, {json: true})
await expectValidationMetadataCalls({cmd_app_validate_json: true})
})

test('outputs JSON issues when active config has TOML parse errors', async () => {
Expand All @@ -71,5 +83,88 @@ describe('app config validate command', () => {

expect(outputResult).toHaveBeenCalledWith(expect.stringContaining('"valid": false'))
expect(linkedAppContext).not.toHaveBeenCalled()
await expectValidationMetadataCalls(
{cmd_app_validate_json: true},
{
cmd_app_validate_valid: false,
cmd_app_validate_issue_count: 1,
cmd_app_validate_file_count: 1,
},
)
})

test('records failure metadata for config errors in non-json mode', async () => {
vi.mocked(Project.load).mockResolvedValue({errors: []} as unknown as Project)
vi.mocked(selectActiveConfig).mockResolvedValue({file: new TomlFile('shopify.app.toml', {})} as any)
vi.mocked(errorsForConfig).mockReturnValue([
{path: '/app/shopify.app.toml', message: 'Missing required field'} as any,
{path: '/app/shopify.app.toml', message: 'Invalid value'} as any,
])

await expect(Validate.run([], import.meta.url)).rejects.toThrow()

expect(linkedAppContext).not.toHaveBeenCalled()
await expectValidationMetadataCalls(
{cmd_app_validate_json: false},
{
cmd_app_validate_valid: false,
cmd_app_validate_issue_count: 2,
cmd_app_validate_file_count: 1,
},
)
})

test('records failure metadata when Project.load fails with --json', async () => {
vi.mocked(Project.load).mockRejectedValue(new AbortError('Could not find app configuration'))

await expect(Validate.run(['--json'], import.meta.url)).rejects.toThrow()

expect(outputResult).toHaveBeenCalledWith(expect.stringContaining('"valid": false'))
expect(selectActiveConfig).not.toHaveBeenCalled()
await expectValidationMetadataCalls(
{cmd_app_validate_json: true},
{
cmd_app_validate_valid: false,
cmd_app_validate_issue_count: 1,
cmd_app_validate_file_count: 1,
},
)
})

test('records failure metadata when selectActiveConfig fails with --json', async () => {
vi.mocked(Project.load).mockResolvedValue({errors: []} as unknown as Project)
vi.mocked(selectActiveConfig).mockRejectedValue(new AbortError('No config found'))

await expect(Validate.run(['--json'], import.meta.url)).rejects.toThrow()

expect(outputResult).toHaveBeenCalledWith(expect.stringContaining('"valid": false'))
expect(linkedAppContext).not.toHaveBeenCalled()
await expectValidationMetadataCalls(
{cmd_app_validate_json: true},
{
cmd_app_validate_valid: false,
cmd_app_validate_issue_count: 1,
cmd_app_validate_file_count: 1,
},
)
})

test('records failure metadata when linkedAppContext throws a validation error with --json', async () => {
vi.mocked(Project.load).mockResolvedValue({errors: []} as unknown as Project)
vi.mocked(selectActiveConfig).mockResolvedValue({file: new TomlFile('shopify.app.toml', {})} as any)
vi.mocked(errorsForConfig).mockReturnValue([])
vi.mocked(linkedAppContext).mockRejectedValue(new AbortError('Validation errors in /app/shopify.app.toml'))

await expect(Validate.run(['--json'], import.meta.url)).rejects.toThrow()

expect(outputResult).toHaveBeenCalledWith(expect.stringContaining('"valid": false'))
await expectValidationMetadataCalls(
{cmd_app_validate_json: true},
{
cmd_app_validate_valid: false,
cmd_app_validate_issue_count: 1,
cmd_app_validate_file_count: 1,
},
)
})
})
18 changes: 18 additions & 0 deletions packages/app/src/cli/commands/app/config/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,20 @@ import {linkedAppContext} from '../../../services/app-context.js'
import {selectActiveConfig} from '../../../models/project/active-config.js'
import {errorsForConfig} from '../../../models/project/config-selection.js'
import {Project} from '../../../models/project/project.js'
import metadata from '../../../metadata.js'
import {globalFlags, jsonFlag} from '@shopify/cli-kit/node/cli'
import {AbortError, AbortSilentError} from '@shopify/cli-kit/node/error'
import {outputResult, stringifyMessage, unstyled} from '@shopify/cli-kit/node/output'
import {renderError} from '@shopify/cli-kit/node/ui'

async function recordValidationFailure(issueCount: number, fileCount: number) {
await metadata.addPublicMetadata(() => ({
cmd_app_validate_valid: false,
cmd_app_validate_issue_count: issueCount,
cmd_app_validate_file_count: fileCount,
}))
}

export default class Validate extends AppLinkedCommand {
static summary = 'Validate your app configuration and extensions.'

Expand All @@ -26,12 +35,17 @@ export default class Validate extends AppLinkedCommand {
public async run(): Promise<AppLinkedCommandOutput> {
const {flags} = await this.parse(Validate)

await metadata.addPublicMetadata(() => ({
cmd_app_validate_json: flags.json,
}))

// Stage 1: Load project
let project: Project
try {
project = await Project.load(flags.path)
} catch (err) {
if (err instanceof AbortError && flags.json) {
await recordValidationFailure(1, 1)
const message = unstyled(stringifyMessage(err.message)).trim()
outputResult(JSON.stringify({valid: false, issues: [{message}]}, null, 2))
throw new AbortSilentError()
Expand All @@ -45,6 +59,7 @@ export default class Validate extends AppLinkedCommand {
activeConfig = await selectActiveConfig(project, flags.config)
} catch (err) {
if (err instanceof AbortError && flags.json) {
await recordValidationFailure(1, 1)
const message = unstyled(stringifyMessage(err.message)).trim()
outputResult(JSON.stringify({valid: false, issues: [{message}]}, null, 2))
throw new AbortSilentError()
Expand All @@ -55,6 +70,8 @@ export default class Validate extends AppLinkedCommand {
const configErrors = errorsForConfig(project, activeConfig.file)
if (configErrors.length > 0) {
const issues = configErrors.map((err) => ({file: err.path, message: err.message}))
const fileCount = new Set(configErrors.map((err) => err.path)).size
await recordValidationFailure(issues.length, fileCount)
if (flags.json) {
outputResult(JSON.stringify({valid: false, issues}, null, 2))
throw new AbortSilentError()
Expand Down Expand Up @@ -83,6 +100,7 @@ export default class Validate extends AppLinkedCommand {
const message = err instanceof AbortError ? unstyled(stringifyMessage(err.message)).trim() : ''
const isValidationError = message.startsWith('Validation errors in ')
if (isValidationError && flags.json) {
await recordValidationFailure(1, 1)
outputResult(JSON.stringify({valid: false, issues: [{message}]}, null, 2))
throw new AbortSilentError()
}
Expand Down
37 changes: 37 additions & 0 deletions packages/app/src/cli/services/validate.test.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import {validateApp} from './validate.js'
import {testAppLinked} from '../models/app/app.test-data.js'
import {AppErrors, formatConfigurationError} from '../models/app/loader.js'
import metadata from '../metadata.js'
import {describe, expect, test, vi} from 'vitest'
import {outputResult} from '@shopify/cli-kit/node/output'
import {renderError, renderSuccess} from '@shopify/cli-kit/node/ui'
import {AbortSilentError} from '@shopify/cli-kit/node/error'

vi.mock('../metadata.js', () => ({default: {addPublicMetadata: vi.fn()}}))
vi.mock('@shopify/cli-kit/node/output', async (importOriginal) => {
const actual = await importOriginal<typeof import('@shopify/cli-kit/node/output')>()
return {
Expand All @@ -15,6 +17,16 @@ vi.mock('@shopify/cli-kit/node/output', async (importOriginal) => {
})
vi.mock('@shopify/cli-kit/node/ui')

async function expectLastValidationMetadata(expected: {
cmd_app_validate_valid: boolean
cmd_app_validate_issue_count: number
cmd_app_validate_file_count: number
}) {
const getMetadata = vi.mocked(metadata.addPublicMetadata).mock.calls.at(-1)?.[0]
expect(getMetadata).toBeDefined()
await expect(Promise.resolve(getMetadata!())).resolves.toEqual(expected)
}

describe('formatConfigurationError', () => {
test('returns plain message when no path', () => {
expect(formatConfigurationError({file: 'foo.toml', message: 'something broke'})).toBe('something broke')
Expand All @@ -34,6 +46,11 @@ describe('validateApp', () => {
expect(renderSuccess).toHaveBeenCalledWith({headline: 'App configuration is valid.'})
expect(renderError).not.toHaveBeenCalled()
expect(outputResult).not.toHaveBeenCalled()
await expectLastValidationMetadata({
cmd_app_validate_valid: true,
cmd_app_validate_issue_count: 0,
cmd_app_validate_file_count: 0,
})
})

test('outputs json success when --json is enabled and there are no errors', async () => {
Expand All @@ -42,6 +59,11 @@ describe('validateApp', () => {
expect(outputResult).toHaveBeenCalledWith(JSON.stringify({valid: true, issues: []}, null, 2))
expect(renderSuccess).not.toHaveBeenCalled()
expect(renderError).not.toHaveBeenCalled()
await expectLastValidationMetadata({
cmd_app_validate_valid: true,
cmd_app_validate_issue_count: 0,
cmd_app_validate_file_count: 0,
})
})

test('renders errors and throws when there are validation errors', async () => {
Expand All @@ -58,6 +80,11 @@ describe('validateApp', () => {
})
expect(renderSuccess).not.toHaveBeenCalled()
expect(outputResult).not.toHaveBeenCalled()
await expectLastValidationMetadata({
cmd_app_validate_valid: false,
cmd_app_validate_issue_count: 2,
cmd_app_validate_file_count: 2,
})
})

test('outputs structured json issues when --json is enabled and there are validation errors', async () => {
Expand All @@ -83,6 +110,11 @@ describe('validateApp', () => {
)
expect(renderError).not.toHaveBeenCalled()
expect(renderSuccess).not.toHaveBeenCalled()
await expectLastValidationMetadata({
cmd_app_validate_valid: false,
cmd_app_validate_issue_count: 2,
cmd_app_validate_file_count: 2,
})
})

test('renders success when errors object exists but is empty', async () => {
Expand Down Expand Up @@ -111,5 +143,10 @@ describe('validateApp', () => {
2,
),
)
await expectLastValidationMetadata({
cmd_app_validate_valid: false,
cmd_app_validate_issue_count: 1,
cmd_app_validate_file_count: 1,
})
})
})
14 changes: 14 additions & 0 deletions packages/app/src/cli/services/validate.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {AppLinkedInterface} from '../models/app/app.js'
import {formatConfigurationError} from '../models/app/loader.js'
import metadata from '../metadata.js'
import {outputResult} from '@shopify/cli-kit/node/output'
import {renderError, renderSuccess} from '@shopify/cli-kit/node/ui'
import {AbortSilentError} from '@shopify/cli-kit/node/error'
Expand All @@ -8,10 +9,22 @@ interface ValidateAppOptions {
json: boolean
}

async function recordValidationMetadata(valid: boolean, errors: {file: string}[]) {
const fileCount = new Set(errors.map((error) => error.file)).size

await metadata.addPublicMetadata(() => ({
cmd_app_validate_valid: valid,
cmd_app_validate_issue_count: errors.length,
cmd_app_validate_file_count: fileCount,
}))
}

export async function validateApp(app: AppLinkedInterface, options: ValidateAppOptions = {json: false}): Promise<void> {
const appErrors = app.errors

if (!appErrors || appErrors.isEmpty()) {
await recordValidationMetadata(true, [])

if (options.json) {
outputResult(JSON.stringify({valid: true, issues: []}, null, 2))
return
Expand All @@ -22,6 +35,7 @@ export async function validateApp(app: AppLinkedInterface, options: ValidateAppO
}

const errors = appErrors.getErrors()
await recordValidationMetadata(false, errors)

if (options.json) {
const issues = errors.map(({file, message, path, code}) => ({file, message, path, code}))
Expand Down
6 changes: 6 additions & 0 deletions packages/cli-kit/src/private/node/analytics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,12 @@ export async function getSensitiveEnvironmentData(config: Interfaces.Config) {
}

function getShopifyEnvironmentVariables() {
// Agent callers can identify themselves today via SHOPIFY_* environment
// variables. The current contract is intentionally lightweight and is kept in
// the sensitive payload until we prove which dimensions deserve first-class
// Monorail fields, e.g. SHOPIFY_CLI_AGENT, SHOPIFY_CLI_AGENT_VERSION,
// SHOPIFY_CLI_AGENT_RUN_ID, SHOPIFY_CLI_AGENT_SESSION_ID, and
// SHOPIFY_CLI_AGENT_PROVIDER.
return Object.fromEntries(Object.entries(process.env).filter(([key]) => key.startsWith('SHOPIFY_')))
}

Expand Down
4 changes: 4 additions & 0 deletions packages/cli-kit/src/public/node/monorail.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ export interface Schemas {
cmd_app_linked_config_uses_cli_managed_urls?: Optional<boolean>
cmd_app_warning_api_key_deprecation_displayed?: Optional<boolean>
cmd_app_deployment_mode?: Optional<string>
cmd_app_validate_json?: Optional<boolean>
cmd_app_validate_valid?: Optional<boolean>
cmd_app_validate_issue_count?: Optional<number>
cmd_app_validate_file_count?: Optional<number>

// Dev related commands
cmd_dev_tunnel_type?: Optional<string>
Expand Down
Loading