Skip to content
Open
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
46 changes: 45 additions & 1 deletion packages/app/src/cli/services/build/extension.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {FunctionConfigType} from '../../models/extensions/specifications/functio
import {beforeEach, describe, expect, test, vi} from 'vitest'
import {exec} from '@shopify/cli-kit/node/system'
import lockfile from 'proper-lockfile'
import {AbortError} from '@shopify/cli-kit/node/error'
import {AbortError, ExternalError} from '@shopify/cli-kit/node/error'
import {fileExistsSync, touchFile, writeFile} from '@shopify/cli-kit/node/fs'
import {joinPath} from '@shopify/cli-kit/node/path'

Expand Down Expand Up @@ -439,4 +439,48 @@ describe('buildFunctionExtension', () => {
expect(touchFile).not.toHaveBeenCalled()
expect(writeFile).not.toHaveBeenCalled()
})

test('preserves stderr details from build command failures in the error tryMessage', async () => {
// Given
// Simulate an ExternalError like exec() throws when cargo/rust is not installed
const externalError = new ExternalError('Command failed with exit code 127: cargo build --release', 'cargo', [
'build',
'--release',
])
vi.mocked(exec).mockRejectedValueOnce(externalError)

// When
const error = await buildFunctionExtension(extension, {
stdout,
stderr,
signal,
app,
environment: 'production',
}).catch((err) => err)

// Then
expect(error).toBeInstanceOf(AbortError)
// The tryMessage should contain the original error details, not just a generic message
expect(error.tryMessage).toContain('Command failed with exit code 127')
expect(error.tryMessage).toContain('cargo build --release')
})

test('preserves generic error messages from build command failures in the error tryMessage', async () => {
// Given
const genericError = new Error('cargo: command not found')
vi.mocked(exec).mockRejectedValueOnce(genericError)

// When
const error = await buildFunctionExtension(extension, {
stdout,
stderr,
signal,
app,
environment: 'production',
}).catch((err) => err)

// Then
expect(error).toBeInstanceOf(AbortError)
expect(error.tryMessage).toContain('cargo: command not found')
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import {AppLinkedInterface, CurrentAppConfiguration} from '../../../models/app/a
import {AppAccessSpecIdentifier} from '../../../models/extensions/specifications/app_config_app_access.js'
import {PosSpecIdentifier} from '../../../models/extensions/specifications/app_config_point_of_sale.js'
import {afterEach, beforeEach, describe, expect, test, vi} from 'vitest'
import {AbortError} from '@shopify/cli-kit/node/error'
import {AbortSignal, AbortController} from '@shopify/cli-kit/node/abort'
import {flushPromises} from '@shopify/cli-kit/node/promises'
import {inTemporaryDirectory} from '@shopify/cli-kit/node/fs'
Expand Down Expand Up @@ -577,6 +578,50 @@ describe('app-event-watcher', () => {
})
})

test('AbortError tryMessage details are included in stderr output', async () => {
await inTemporaryDirectory(async (tmpDir) => {
// Use a fresh extension to avoid interference from other tests sharing flowExtension
const freshExtension = await testFlowActionExtension('/extensions/flow_action_fresh')
const fileWatchEvent: WatcherEvent = {
type: 'file_updated',
path: '/extensions/flow_action_fresh/src/file.js',
extensionPath: '/extensions/flow_action_fresh',
startTime: [0, 0],
}

// Given
// This simulates the error thrown by buildFunctionExtension when cargo/rust fails:
// AbortError('Failed to build function.', 'Command failed with exit code 127: cargo build --release')
const buildError = new AbortError(
'Failed to build function.',
'Command failed with exit code 127: cargo build --release',
)
freshExtension.buildForBundle = vi.fn().mockRejectedValueOnce(buildError)

const buildOutputPath = joinPath(tmpDir, '.shopify', 'bundle')
const app = testAppLinked({
allExtensions: [freshExtension],
configPath: 'shopify.app.custom.toml',
configuration: testAppConfiguration,
})

// When
const mockManager = new MockESBuildContextManager()
const mockFileWatcher = new MockFileWatcher(app, outputOptions, [fileWatchEvent])
const watcher = new AppEventWatcher(app, 'url', buildOutputPath, mockManager, mockFileWatcher)
const stderr = {write: vi.fn()} as unknown as Writable
const stdout = {write: vi.fn()} as unknown as Writable

await watcher.start({stdout, stderr, signal: abortController.signal})

await flushPromises()

// Then - stderr should include the tryMessage with the actual failure reason
const allStderrWrites = (stderr.write as any).mock.calls.map((call: any[]) => call[0]).join('\n')
expect(allStderrWrites).toContain('Command failed with exit code 127')
})
})

test('uncaught errors are emitted', async () => {
await inTemporaryDirectory(async (tmpDir) => {
const fileWatchEvent: WatcherEvent = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,11 @@ export class AppEventWatcher extends EventEmitter {
})
} else {
this.options.stderr.write(error.message)
if (error.tryMessage) {
this.options.stderr.write(
typeof error.tryMessage === 'string' ? error.tryMessage : String(error.tryMessage),
)
Comment on lines +292 to +294
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When printing error.message and then error.tryMessage, the two writes are concatenated with no separator, so users may see a single glued string (e.g., Failed to build function.Command failed ...). Consider writing a newline (or \n\n) between them, or only adding a separator if error.message doesn’t already end with one, so the output is readable.

Suggested change
this.options.stderr.write(
typeof error.tryMessage === 'string' ? error.tryMessage : String(error.tryMessage),
)
const tryMessage =
typeof error.tryMessage === 'string' ? error.tryMessage : String(error.tryMessage)
if (!String(error.message).endsWith('\n')) {
this.options.stderr.write('\n')
}
this.options.stderr.write(tryMessage)

Copilot uses AI. Check for mistakes.
}
Comment on lines +291 to +295
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AbortError.tryMessage is typed as a TokenItem | null (not necessarily a plain string). Using String(error.tryMessage) can yield [object Object] for tokenized messages. Since this file already imports from @shopify/cli-kit/node/output, consider using itemToString(error.tryMessage) (or equivalent) to reliably render token items to a human-readable string before writing to stderr.

Copilot uses AI. Check for mistakes.
}

// Update all events for this extension with error result
Expand Down
Loading