Skip to content
Closed
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
5 changes: 5 additions & 0 deletions .changeset/soft-schema-compilation-failures.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/app': patch
---

Fall back to local validation instead of crashing when a server-provided module validation schema cannot be compiled
78 changes: 78 additions & 0 deletions packages/app/src/cli/utilities/json-schema.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {unifiedConfigurationParserFactory} from './json-schema.js'
import {describe, test, expect} from 'vitest'
import {randomUUID} from '@shopify/cli-kit/node/crypto'
import {mockAndCaptureOutput} from '@shopify/cli-kit/node/testing/output'

describe('unifiedConfigurationParserFactory', () => {
const mockParseConfigurationObject = (config: any) => {
Expand Down Expand Up @@ -185,4 +186,81 @@ describe('unifiedConfigurationParserFactory', () => {
custom: 'value',
})
})

test('falls back to the zod result when the contract fails to compile (e.g. empty enum)', async () => {
// Given: the shape a server renders when its template registry is empty —
// an empty enum is invalid JSON Schema and AJV refuses to compile it.
const invalidContract = JSON.stringify({
type: 'object',
properties: {
metaobjects: {
type: 'array',
items: {type: 'string', enum: []},
},
},
})
const merged = {
identifier: randomUUID(),
parseConfigurationObject: mockParseConfigurationObject,
}

// When
const parser = await unifiedConfigurationParserFactory(merged as any, {jsonSchema: invalidContract})
const mockOutput = mockAndCaptureOutput()
const result = parser({type: 'product_subscription'})

// Then: no throw, zod result served, warning emitted once
expect(result).toEqual({
state: 'ok',
data: {type: 'product_subscription'},
errors: undefined,
})
expect(mockOutput.warn()).toContain(`The validation schema provided for "${merged.identifier}"`)

mockOutput.clear()
parser({type: 'product_subscription'})
expect(mockOutput.warn()).toBe('')
})

test('still reports zod errors when the contract fails to compile', async () => {
// Given
const invalidContract = JSON.stringify({
type: 'object',
properties: {anything: {type: 'string', enum: []}},
})
const merged = {
identifier: randomUUID(),
parseConfigurationObject: mockParseConfigurationObject,
}

// When
const parser = await unifiedConfigurationParserFactory(merged as any, {jsonSchema: invalidContract})
const result = parser({type: 'invalid'})

// Then: local validation still gates the config
expect(result.state).toBe('error')
expect(result.errors).toEqual([{path: ['type'], message: 'Invalid type'}])
})

test('falls back to the zod parser when the contract cannot be normalised (broken $ref)', async () => {
// Given
const brokenRefContract = JSON.stringify({
type: 'object',
properties: {thing: {$ref: '#/definitions/DoesNotExist'}},
})
const merged = {
identifier: randomUUID(),
parseConfigurationObject: mockParseConfigurationObject,
}

// When
const mockOutput = mockAndCaptureOutput()
const parser = await unifiedConfigurationParserFactory(merged as any, {jsonSchema: brokenRefContract})
const result = parser({type: 'product_subscription'})

// Then: the factory degrades to the local parser instead of throwing
expect(parser).toBe(merged.parseConfigurationObject)
expect(result.state).toBe('ok')
expect(mockOutput.warn()).toContain(`The validation schema provided for "${merged.identifier}"`)
})
})
56 changes: 48 additions & 8 deletions packages/app/src/cli/utilities/json-schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
} from '@shopify/cli-kit/node/json-schema'
import {isEmpty} from '@shopify/cli-kit/common/object'
import {JsonMapType} from '@shopify/cli-kit/node/toml'
import {outputWarn} from '@shopify/cli-kit/node/output'

/**
* The base properties that are added to all JSON Schema contracts.
Expand Down Expand Up @@ -39,10 +40,20 @@ export async function unifiedConfigurationParserFactory(
if (contractJsonSchema === undefined || isEmpty(JSON.parse(contractJsonSchema))) {
return merged.parseConfigurationObject
}
const contract = await normaliseJsonSchema(contractJsonSchema)
contract.properties = {...JsonSchemaBaseProperties, ...contract.properties}
const extensionIdentifier = merged.identifier

let contract: Awaited<ReturnType<typeof normaliseJsonSchema>>
try {
contract = await normaliseJsonSchema(contractJsonSchema)
// eslint-disable-next-line no-catch-all/no-catch-all
} catch (error) {
// A server-provided contract the CLI cannot use must not take down every
// app command: fall back to local validation for this module only.
warnAboutUnusableContract(extensionIdentifier, error)
return merged.parseConfigurationObject
}
contract.properties = {...JsonSchemaBaseProperties, ...contract.properties}

const parseConfigurationObject = (config: object): ParseConfigurationResult<BaseConfigType> => {
// First we parse with zod. This may also change the format of the data.
const zodParse = merged.parseConfigurationObject(config)
Expand All @@ -51,12 +62,22 @@ export async function unifiedConfigurationParserFactory(
const zodValidatedData = zodParse.state === 'ok' ? zodParse.data : undefined
const subjectForAjv = zodValidatedData ?? (config as JsonMapType)

const jsonSchemaParse = jsonSchemaValidate(
subjectForAjv,
contract,
handleInvalidAdditionalProperties,
extensionIdentifier,
)
let jsonSchemaParse
try {
jsonSchemaParse = jsonSchemaValidate(
subjectForAjv,
contract,
handleInvalidAdditionalProperties,
extensionIdentifier,
)
// eslint-disable-next-line no-catch-all/no-catch-all
} catch (error) {
// Schema compilation happens on first use: an invalid contract (e.g. an
// empty enum rendered by a server with no data for it) throws here.
// Degrade to the zod result instead of crashing app loading.
warnAboutUnusableContract(extensionIdentifier, error)
return zodParse
}

// Finally, we de-duplicate the error set from both validations -- identical messages for identical paths are removed
let errors = zodParse.errors ?? []
Expand Down Expand Up @@ -88,3 +109,22 @@ export async function unifiedConfigurationParserFactory(
}
return parseConfigurationObject
}

const warnedContractIdentifiers = new Set<string>()

/**
* Warn (once per module identifier per process) that a server-provided
* contract could not be used, without failing the command.
*
* @param extensionIdentifier - The module whose contract is unusable.
* @param error - The underlying normalisation/compilation error.
*/
function warnAboutUnusableContract(extensionIdentifier: string, error: unknown) {
if (warnedContractIdentifiers.has(extensionIdentifier)) return
warnedContractIdentifiers.add(extensionIdentifier)
const reason = error instanceof Error ? error.message : String(error)
outputWarn(
`The validation schema provided for "${extensionIdentifier}" couldn't be used and was ignored (${reason}). ` +
`Validation for this configuration falls back to the CLI's local schema; the server still validates on deploy.`,
)
}
Loading