From 9e31ede4672c67ae6cc3b88b098fee1030fbb855 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Mon, 20 Jul 2026 17:12:52 +0200 Subject: [PATCH] Send analytics in background so the main command can finish early --- packages/app/src/cli/commands/app/dev.ts | 4 + .../src/cli/commands/app/function/replay.ts | 4 + packages/app/src/cli/commands/app/init.ts | 4 + .../cli-kit/src/private/node/analytics.ts | 1 + .../src/private/node/otel-metrics.test.ts | 39 ++++ .../cli-kit/src/private/node/otel-metrics.ts | 5 +- .../cli-kit/src/public/node/analytics.test.ts | 185 +++++++++++++++++- packages/cli-kit/src/public/node/analytics.ts | 144 ++++++++++---- .../cli-kit/src/public/node/base-command.ts | 4 + .../cli-kit/src/public/node/hooks/postrun.ts | 4 +- packages/cli-kit/src/public/node/metadata.ts | 1 + .../public/node/notifications-system.test.ts | 4 +- .../src/public/node/notifications-system.ts | 5 +- .../cli-kit/src/public/node/system.test.ts | 55 +++++- packages/cli-kit/src/public/node/system.ts | 12 +- .../node/vendor/otel-js/service/types.ts | 3 +- packages/cli/oclif.manifest.json | 18 ++ .../cli/src/cli/commands/send-analytics.ts | 10 + packages/cli/src/index.ts | 2 + packages/theme/src/cli/commands/theme/dev.ts | 4 + 20 files changed, 453 insertions(+), 55 deletions(-) create mode 100644 packages/cli/src/cli/commands/send-analytics.ts diff --git a/packages/app/src/cli/commands/app/dev.ts b/packages/app/src/cli/commands/app/dev.ts index ee6f217ee97..387824ba0ac 100644 --- a/packages/app/src/cli/commands/app/dev.ts +++ b/packages/app/src/cli/commands/app/dev.ts @@ -11,6 +11,10 @@ import {globalFlags, portFlag} from '@shopify/cli-kit/node/cli' import {addPublicMetadata} from '@shopify/cli-kit/node/metadata' export default class Dev extends AppLinkedCommand { + public static get requiresSyncAnalytics(): boolean { + return true + } + static summary = 'Run the app.' static descriptionWithMarkdown = `Builds and previews your app on a dev store, and watches for changes. [Read more about testing apps locally](https://shopify.dev/docs/apps/build/cli-for-apps/test-apps-locally).` diff --git a/packages/app/src/cli/commands/app/function/replay.ts b/packages/app/src/cli/commands/app/function/replay.ts index f0d2b4bcfec..c7c99d9b198 100644 --- a/packages/app/src/cli/commands/app/function/replay.ts +++ b/packages/app/src/cli/commands/app/function/replay.ts @@ -7,6 +7,10 @@ import {globalFlags, jsonFlag, requiredIfNonInteractive} from '@shopify/cli-kit/ import {Flags} from '@oclif/core' export default class FunctionReplay extends AppLinkedCommand { + public static get requiresSyncAnalytics(): boolean { + return true + } + static summary = 'Replays a function run from an app log.' static descriptionWithMarkdown = `Runs the function from your current directory for [testing purposes](https://shopify.dev/docs/apps/functions/testing-and-debugging). To learn how you can monitor and debug functions when errors occur, refer to [Shopify Functions error handling](https://shopify.dev/docs/api/functions/errors).` diff --git a/packages/app/src/cli/commands/app/init.ts b/packages/app/src/cli/commands/app/init.ts index 03baa9ed3df..6272a0d31cc 100644 --- a/packages/app/src/cli/commands/app/init.ts +++ b/packages/app/src/cli/commands/app/init.ts @@ -21,6 +21,10 @@ import {AbortError} from '@shopify/cli-kit/node/error' import type {NonTTYFlagRequirement} from '@shopify/cli-kit/node/base-command' export default class Init extends AppLinkedCommand { + public static get requiresSyncAnalytics(): boolean { + return true + } + static summary?: string | undefined = 'Create a new app project' static examples = [ diff --git a/packages/cli-kit/src/private/node/analytics.ts b/packages/cli-kit/src/private/node/analytics.ts index feccd613325..0e0b5df491d 100644 --- a/packages/cli-kit/src/private/node/analytics.ts +++ b/packages/cli-kit/src/private/node/analytics.ts @@ -41,6 +41,7 @@ export async function startAnalytics({ startTime: currentTime, startCommand, startArgs: args, + requiresSyncAnalytics: (commandClass as typeof BaseCommand | undefined)?.requiresSyncAnalytics ?? false, }, })) diff --git a/packages/cli-kit/src/private/node/otel-metrics.test.ts b/packages/cli-kit/src/private/node/otel-metrics.test.ts index 0209846d62a..8387dcc0329 100644 --- a/packages/cli-kit/src/private/node/otel-metrics.test.ts +++ b/packages/cli-kit/src/private/node/otel-metrics.test.ts @@ -26,11 +26,13 @@ describe('otel-metrics', () => { test('logs metrics when activated', async () => { const mockOtelRecorder = vi.fn() + const mockForceFlush = vi.fn().mockResolvedValue(undefined) const mockOtelCreator = vi.fn() mockOtelCreator.mockReturnValue({ type: 'otel', otel: { record: mockOtelRecorder, + getMeterProvider: () => ({forceFlush: mockForceFlush}), }, }) @@ -52,5 +54,42 @@ describe('otel-metrics', () => { expect(mockOtelCreator).toHaveBeenCalledOnce() expect(mockOtelRecorder.mock.calls).toMatchSnapshot() + expect(mockForceFlush).toHaveBeenCalledOnce() + }) + + test('waits for metrics to flush', async () => { + let resolveFlush: () => void = () => {} + const flush = new Promise((resolve) => { + resolveFlush = resolve + }) + const recorderFactory = vi.fn().mockReturnValue({ + type: 'otel', + otel: { + record: vi.fn(), + getMeterProvider: () => ({forceFlush: () => flush}), + }, + }) + + let metricsRecorded = false + const recording = recordMetrics( + { + skipMetricAnalytics: false, + cliVersion: '4.6.0', + owningPlugin: '@shopify/app', + command: 'app dev', + exitMode: 'ok', + }, + {active: 10, network: 20, prompt: 30}, + recorderFactory, + ).then(() => { + metricsRecorded = true + }) + + await vi.waitFor(() => expect(recorderFactory).toHaveBeenCalledOnce()) + expect(metricsRecorded).toBe(false) + + resolveFlush() + await recording + expect(metricsRecorded).toBe(true) }) }) diff --git a/packages/cli-kit/src/private/node/otel-metrics.ts b/packages/cli-kit/src/private/node/otel-metrics.ts index 5e0545503a9..ccef5e155ff 100644 --- a/packages/cli-kit/src/private/node/otel-metrics.ts +++ b/packages/cli-kit/src/private/node/otel-metrics.ts @@ -11,7 +11,7 @@ type MetricRecorder = | 'console' | { type: 'otel' - otel: Pick + otel: Pick } // this should be type, not interface @@ -80,6 +80,9 @@ export async function recordMetrics( recordCommandCounter(recorder, labels) recordCommandTiming(recorder, labels, timing) + if (recorder !== 'console') { + await recorder.otel.getMeterProvider().forceFlush({}) + } } const COMMAND_DURATION_BOUNDARIES_MS = [ diff --git a/packages/cli-kit/src/public/node/analytics.test.ts b/packages/cli-kit/src/public/node/analytics.test.ts index 61d8f67ca19..15f64abd05b 100644 --- a/packages/cli-kit/src/public/node/analytics.test.ts +++ b/packages/cli-kit/src/public/node/analytics.test.ts @@ -1,4 +1,11 @@ -import {reportAnalyticsEvent, recordTiming, recordError, recordRetry, recordEvent} from './analytics.js' +import { + reportAnalyticsEvent, + sendAnalyticsEventFromStdin, + recordTiming, + recordError, + recordRetry, + recordEvent, +} from './analytics.js' import * as os from './os.js' import { analyticsDisabled, @@ -16,12 +23,13 @@ import {mockAndCaptureOutput} from './testing/output.js' import {addPublicMetadata, addSensitiveMetadata} from './metadata.js' import {sendErrorToBugsnag} from './error-handler.js' import {hashString} from './crypto.js' +import {exec, isInsideContainer, readStdinString} from './system.js' import * as store from '../../private/node/analytics/storage.js' import {startAnalytics} from '../../private/node/analytics.js' import {CLI_KIT_VERSION} from '../common/version.js' import {setLastSeenAuthMethod, setLastSeenUserIdAfterAuth} from '../../private/node/session.js' - import {test, expect, describe, vi, beforeEach, afterEach, MockedFunction} from 'vitest' +import type BaseCommand from './base-command.js' vi.mock('./context/local.js') vi.mock('./os.js') @@ -32,6 +40,7 @@ vi.mock('../../version.js') vi.mock('./monorail.js') vi.mock('./cli.js') vi.mock('./error-handler.js') +vi.mock('./system.js') function restoreEnvVariable(key: string, value: string | undefined): void { if (value === undefined) { @@ -44,19 +53,22 @@ function restoreEnvVariable(key: string, value: string | undefined): void { describe('event tracking', () => { const currentDate = new Date(Date.UTC(2022, 1, 1, 10, 0, 0)) let publishEventMock: MockedFunction + let execMock: MockedFunction beforeEach(() => { vi.setSystemTime(currentDate) vi.mocked(isShopify).mockResolvedValue(false) vi.mocked(isDevelopment).mockReturnValue(false) vi.mocked(analyticsDisabled).mockReturnValue(false) - vi.mocked(ciPlatform).mockReturnValue({isCI: true, name: 'vitest', metadata: {}}) + vi.mocked(ciPlatform).mockReturnValue({isCI: false}) vi.mocked(macAddress).mockResolvedValue('macAddress') vi.mocked(hashString).mockReturnValue('hashed-macaddress') vi.mocked(isUnitTest).mockReturnValue(true) vi.mocked(cloudEnvironment).mockReturnValue({platform: 'localhost', editor: false}) vi.mocked(os.platformAndArch).mockReturnValue({platform: 'darwin', arch: 'arm64'}) + vi.mocked(isInsideContainer).mockReturnValue(false) publishEventMock = vi.mocked(publishMonorailEvent).mockReturnValue(Promise.resolve({type: 'ok'})) + execMock = vi.mocked(exec).mockResolvedValue(undefined) }) afterEach(() => { @@ -72,6 +84,167 @@ describe('event tracking', () => { }) } + async function sendReportedAnalyticsPayload(): Promise { + expect(execMock).toHaveBeenCalledOnce() + expect(execMock.mock.calls[0]![0]).toBe(process.execPath) + const execArgs = execMock.mock.calls[0]![1] + expect(execArgs.slice(1)).toEqual(['send-analytics']) + + const payloadInput = execMock.mock.calls[0]![2]?.input + if (payloadInput === undefined) throw new Error('Expected send-analytics to receive stdin input') + + vi.mocked(readStdinString).mockResolvedValueOnce(payloadInput) + await sendAnalyticsEventFromStdin() + } + + test('sends analytics in-process on Windows', async () => { + await inProjectWithFile('package.json', async (args) => { + // Given + const commandContent = {command: 'info', topic: 'app'} + await startAnalytics({commandContent, args, currentTime: currentDate.getTime() - 100}) + vi.mocked(os.platformAndArch).mockReturnValue({platform: 'windows', arch: 'arm64'}) + + const config = { + runHook: vi.fn().mockResolvedValue({successes: [], failures: []}), + plugins: [], + } as any + + // When + await reportAnalyticsEvent({config, exitMode: 'expected_error'}) + + // Then + expect(execMock).not.toHaveBeenCalled() + expect(publishEventMock).toHaveBeenCalledOnce() + }) + }) + + test('does not wait for the analytics process on non-Windows platforms', async () => { + await inProjectWithFile('package.json', async (args) => { + // Given + const commandContent = {command: 'info', topic: 'app'} + await startAnalytics({commandContent, args, currentTime: currentDate.getTime() - 100}) + + let resolveAnalyticsProcess: () => void = () => {} + const analyticsProcess = new Promise((resolve) => { + resolveAnalyticsProcess = resolve + }) + execMock.mockReturnValueOnce(analyticsProcess) + + const config = { + runHook: vi.fn().mockResolvedValue({successes: [], failures: []}), + plugins: [], + } as any + + // When + await reportAnalyticsEvent({config, exitMode: 'expected_error'}) + + // Then + expect(execMock).toHaveBeenCalledWith( + expect.anything(), + expect.anything(), + expect.objectContaining({background: true, input: expect.any(String)}), + ) + resolveAnalyticsProcess() + await sendReportedAnalyticsPayload() + }) + }) + + test('sends analytics in-process in CI', async () => { + await inProjectWithFile('package.json', async (args) => { + // Given + const commandContent = {command: 'info', topic: 'app'} + await startAnalytics({commandContent, args, currentTime: currentDate.getTime() - 100}) + vi.mocked(ciPlatform).mockReturnValue({isCI: true, name: 'github', metadata: {}}) + + const config = { + runHook: vi.fn().mockResolvedValue({successes: [], failures: []}), + plugins: [], + } as any + + // When + await reportAnalyticsEvent({config, exitMode: 'ok'}) + + // Then + expect(execMock).not.toHaveBeenCalled() + expect(publishEventMock).toHaveBeenCalledOnce() + }) + }) + + test('sends analytics in-process inside a container', async () => { + await inProjectWithFile('package.json', async (args) => { + // Given + const commandContent = {command: 'info', topic: 'app'} + await startAnalytics({commandContent, args, currentTime: currentDate.getTime() - 100}) + vi.mocked(isInsideContainer).mockReturnValue(true) + + const config = { + runHook: vi.fn().mockResolvedValue({successes: [], failures: []}), + plugins: [], + } as any + + // When + await reportAnalyticsEvent({config, exitMode: 'ok'}) + + // Then + expect(execMock).not.toHaveBeenCalled() + expect(publishEventMock).toHaveBeenCalledOnce() + }) + }) + + test('sends analytics in-process when required by the command', async () => { + await inProjectWithFile('package.json', async (args) => { + // Given + const commandContent = {command: 'init'} + const commandClass = {requiresSyncAnalytics: true} as unknown as typeof BaseCommand + await startAnalytics({commandContent, args, commandClass, currentTime: currentDate.getTime() - 100}) + const config = { + runHook: vi.fn().mockResolvedValue({successes: [], failures: []}), + plugins: [], + } as any + + // When + await reportAnalyticsEvent({config, exitMode: 'ok'}) + + // Then + expect(execMock).not.toHaveBeenCalled() + expect(publishEventMock).toHaveBeenCalledOnce() + }) + }) + + test('skips send-analytics before building a payload', async () => { + // Given + await startAnalytics({commandContent: {command: 'send-analytics'}, args: []}) + const config = { + runHook: vi.fn(() => { + throw new Error('Analytics hooks should not run') + }), + plugins: [], + } as any + + // When + await reportAnalyticsEvent({config, exitMode: 'ok'}) + + // Then + expect(config.runHook).not.toHaveBeenCalled() + expect(execMock).not.toHaveBeenCalled() + expect(publishEventMock).not.toHaveBeenCalled() + }) + + test('reports invalid analytics JSON received from stdin', async () => { + // Given + vi.mocked(readStdinString).mockResolvedValueOnce('{invalid') + const outputMock = mockAndCaptureOutput() + + // When + await sendAnalyticsEventFromStdin() + + // Then + expect(outputMock.debug()).toContain('Failed to send analytics in background') + expect(publishEventMock).not.toHaveBeenCalled() + expect(sendErrorToBugsnag).toHaveBeenCalledOnce() + expect(sendErrorToBugsnag).toHaveBeenCalledWith(expect.any(Error), 'expected_error') + }) + test('sends the expected data to Monorail with cached app info', async () => { await inProjectWithFile('package.json', async (args) => { // Given @@ -95,6 +268,7 @@ describe('event tracking', () => { plugins: pluginsMap, } as any await reportAnalyticsEvent({config, exitMode: 'ok'}) + await sendReportedAnalyticsPayload() // Then const version = CLI_KIT_VERSION const expectedPayloadPublic = { @@ -155,6 +329,7 @@ describe('event tracking', () => { plugins: [], } as any await reportAnalyticsEvent({config, exitMode: 'ok'}) + await sendReportedAnalyticsPayload() // Then expect(publishEventMock).toHaveBeenCalledOnce() @@ -179,6 +354,7 @@ describe('event tracking', () => { plugins: [], } as any await reportAnalyticsEvent({config, errorMessage: 'Permission denied', exitMode: 'unexpected_error'}) + await sendReportedAnalyticsPayload() // Then const version = CLI_KIT_VERSION @@ -219,6 +395,7 @@ describe('event tracking', () => { plugins: [], } as any await reportAnalyticsEvent({config, exitMode: 'ok'}) + await sendReportedAnalyticsPayload() // Then const expectedPayloadSensitive = { @@ -243,6 +420,7 @@ describe('event tracking', () => { plugins: [], } as any await reportAnalyticsEvent({config, exitMode: 'ok'}) + await sendReportedAnalyticsPayload() expect(publishEventMock).toHaveBeenCalledOnce() expect(publishEventMock.mock.calls[0]![2]).toMatchObject({ @@ -274,6 +452,7 @@ describe('event tracking', () => { plugins: [], } as any await reportAnalyticsEvent({config, exitMode: 'ok'}) + await sendReportedAnalyticsPayload() // Then const sensitivePayload = publishEventMock.mock.calls[0]![2] diff --git a/packages/cli-kit/src/public/node/analytics.ts b/packages/cli-kit/src/public/node/analytics.ts index dcaef13d51c..d20f7c90a8f 100644 --- a/packages/cli-kit/src/public/node/analytics.ts +++ b/packages/cli-kit/src/public/node/analytics.ts @@ -1,6 +1,6 @@ -import {alwaysLogAnalytics, alwaysLogMetrics, analyticsDisabled, isShopify} from './context/local.js' +import {alwaysLogAnalytics, alwaysLogMetrics, analyticsDisabled, ciPlatform, isShopify} from './context/local.js' import * as metadata from './metadata.js' -import {publishMonorailEvent, MONORAIL_COMMAND_TOPIC} from './monorail.js' +import {publishMonorailEvent, MONORAIL_COMMAND_TOPIC, type Schemas} from './monorail.js' import {fanoutHooks} from './plugins.js' import {sendErrorToBugsnag} from './error-handler.js' import {outputContent, outputDebug, outputToken} from './output.js' @@ -36,6 +36,70 @@ interface ReportAnalyticsEventOptions { exitMode: CommandExitMode } +type MonorailCommandPayload = Schemas[typeof MONORAIL_COMMAND_TOPIC] +interface AnalyticsPayload { + public: MonorailCommandPayload['public'] & {cmd_all_exit: CommandExitMode} + sensitive: MonorailCommandPayload['sensitive'] +} + +async function sendAnalyticsEvent( + payload: AnalyticsPayload, + skipMonorailAnalytics: boolean, + skipMetricAnalytics: boolean, +): Promise { + const doMonorail = async () => { + if (skipMonorailAnalytics) return + const response = await publishMonorailEvent(MONORAIL_COMMAND_TOPIC, payload.public, payload.sensitive) + if (response.type === 'error') { + outputDebug(response.message) + } + } + + const doOpenTelemetry = async () => { + const active = payload.public.cmd_all_timing_active_ms ?? 0 + const network = payload.public.cmd_all_timing_network_ms ?? 0 + const prompt = payload.public.cmd_all_timing_prompts_ms ?? 0 + + return recordMetrics( + { + skipMetricAnalytics, + cliVersion: payload.public.cli_version, + owningPlugin: payload.public.cmd_all_plugin ?? '@shopify/cli', + command: payload.public.command, + exitMode: payload.public.cmd_all_exit, + }, + { + active, + network, + prompt, + }, + ) + } + + await Promise.all([doMonorail(), doOpenTelemetry()]) +} + +export async function sendAnalyticsEventFromStdin(): Promise { + try { + const {readStdinString} = await import('./system.js') + const payloadStr = await readStdinString() + if (payloadStr === undefined) throw new Error('No analytics payload received from stdin') + + const {payload, skipMonorailAnalytics, skipMetricAnalytics} = JSON.parse(payloadStr) as { + payload: AnalyticsPayload + skipMonorailAnalytics: boolean + skipMetricAnalytics: boolean + } + + await sendAnalyticsEvent(payload, skipMonorailAnalytics, skipMetricAnalytics) + // eslint-disable-next-line no-catch-all/no-catch-all + } catch (error) { + const message = error instanceof Error ? error.message : String(error) + outputDebug(`Failed to send analytics in background: ${message}`) + await sendErrorToBugsnag(error, 'expected_error') + } +} + /** * Report an analytics event, sending it off to Monorail -- Shopify's internal analytics service. * @@ -44,11 +108,11 @@ interface ReportAnalyticsEventOptions { */ export async function reportAnalyticsEvent(options: ReportAnalyticsEventOptions): Promise { try { + const commandStartOptions = metadata.getAllSensitiveMetadata().commandStartOptions + if (commandStartOptions?.startCommand === 'send-analytics') return + const payload = await buildPayload(options) - if (payload === undefined) { - // Nothing to log - return - } + if (payload === undefined) return let withinRateLimit = false await runWithRateLimit({ @@ -65,40 +129,42 @@ export async function reportAnalyticsEvent(options: ReportAnalyticsEventOptions) const skipMonorailAnalytics = !alwaysLogAnalytics() && analyticsDisabled() const skipMetricAnalytics = !alwaysLogMetrics() && analyticsDisabled() - if (skipMonorailAnalytics || skipMetricAnalytics) { + if (skipMonorailAnalytics && skipMetricAnalytics) { outputDebug(outputContent`Skipping command analytics, payload: ${outputToken.json(payload)}`) + return } - const doMonorail = async () => { - if (skipMonorailAnalytics) { - return - } - const response = await publishMonorailEvent(MONORAIL_COMMAND_TOPIC, payload.public, payload.sensitive) - if (response.type === 'error') { - outputDebug(response.message) - } - } - const doOpenTelemetry = async () => { - const active = payload.public.cmd_all_timing_active_ms ?? 0 - const network = payload.public.cmd_all_timing_network_ms ?? 0 - const prompt = payload.public.cmd_all_timing_prompts_ms ?? 0 - - return recordMetrics( - { - skipMetricAnalytics, - cliVersion: payload.public.cli_version, - owningPlugin: payload.public.cmd_all_plugin ?? '@shopify/cli', - command: payload.public.command, - exitMode: options.exitMode, - }, - { - active, - network, - prompt, - }, - ) + const [{platformAndArch}, {isInsideContainer}] = await Promise.all([import('./os.js'), import('./system.js')]) + const sendInBackground = + !commandStartOptions?.requiresSyncAnalytics && + platformAndArch().platform !== 'windows' && + !ciPlatform().isCI && + !isInsideContainer() + const deliveryDescription = sendInBackground ? ' in background' : '' + outputDebug(outputContent`Sending command analytics${deliveryDescription}, payload: ${outputToken.json(payload)}`) + + if (!sendInBackground) { + await sendAnalyticsEvent(payload, skipMonorailAnalytics, skipMetricAnalytics) + return } - await Promise.all([doMonorail(), doOpenTelemetry()]) + + const {exec} = await import('./system.js') + const argv = process.argv + if (!argv[1]) return + const nodeBinary = process.execPath + const shopifyBinary = argv[1] + const args = [shopifyBinary, 'send-analytics'] + + const analyticsProcess = exec(nodeBinary, args, { + background: true, + env: {...process.env, SHOPIFY_CLI_NO_ANALYTICS: '1'}, + input: JSON.stringify({payload, skipMonorailAnalytics, skipMetricAnalytics}), + externalErrorHandler: async (error: unknown) => { + outputDebug(`Failed to send analytics in background: ${(error as Error).message}`) + }, + }) + // eslint-disable-next-line no-void + void analyticsProcess // eslint-disable-next-line no-catch-all/no-catch-all } catch (error) { @@ -111,7 +177,11 @@ export async function reportAnalyticsEvent(options: ReportAnalyticsEventOptions) } } -async function buildPayload({config, errorMessage, exitMode}: ReportAnalyticsEventOptions) { +async function buildPayload({ + config, + errorMessage, + exitMode, +}: ReportAnalyticsEventOptions): Promise { const {commandStartOptions, environmentFlags, ...sensitiveMetadata} = metadata.getAllSensitiveMetadata() if (commandStartOptions === undefined) { outputDebug('Unable to log analytics event - no information on executed command') diff --git a/packages/cli-kit/src/public/node/base-command.ts b/packages/cli-kit/src/public/node/base-command.ts index c81438e52ff..ec80b0ad220 100644 --- a/packages/cli-kit/src/public/node/base-command.ts +++ b/packages/cli-kit/src/public/node/base-command.ts @@ -33,6 +33,10 @@ interface EnvironmentFlags { abstract class BaseCommand extends Command { static baseFlags: FlagInput<{}> = {} + public static get requiresSyncAnalytics(): boolean { + return false + } + public static nonTTYFlagRequirements(_flags: FlagOutput): NonTTYFlagRequirement[] { return [] } diff --git a/packages/cli-kit/src/public/node/hooks/postrun.ts b/packages/cli-kit/src/public/node/hooks/postrun.ts index 8718e50218e..08d2afd6518 100644 --- a/packages/cli-kit/src/public/node/hooks/postrun.ts +++ b/packages/cli-kit/src/public/node/hooks/postrun.ts @@ -74,11 +74,11 @@ export const hook: Hook.Postrun = async ({config, Command}) => { const command = Command.id.replace(/:/g, ' ') outputDebug(`Completed command ${command}`) - if (!command.includes('notifications') && !command.includes('upgrade')) await autoUpgradeIfNeeded() + if (!command.includes('notifications') && !command.includes('upgrade') && !command.includes('send-analytics')) + await autoUpgradeIfNeeded() const {reportAnalyticsEvent} = await import('../analytics.js') await reportAnalyticsEvent({config, exitMode: 'ok'}) - postRunHookCompleted = true } diff --git a/packages/cli-kit/src/public/node/metadata.ts b/packages/cli-kit/src/public/node/metadata.ts index 30e05719e55..2530ab6d96b 100644 --- a/packages/cli-kit/src/public/node/metadata.ts +++ b/packages/cli-kit/src/public/node/metadata.ts @@ -193,6 +193,7 @@ const coreData = createRuntimeMetadataContainer< startCommand: string startTopic?: string startArgs: string[] + requiresSyncAnalytics?: boolean } } & {environmentFlags: string} & PickByPrefix >({cmd_all_timing_network_ms: 0, cmd_all_timing_prompts_ms: 0}) diff --git a/packages/cli-kit/src/public/node/notifications-system.test.ts b/packages/cli-kit/src/public/node/notifications-system.test.ts index 52e68616d8b..9dab5a616d6 100644 --- a/packages/cli-kit/src/public/node/notifications-system.test.ts +++ b/packages/cli-kit/src/public/node/notifications-system.test.ts @@ -445,7 +445,7 @@ describe('fetchNotificationsInBackground', () => { expect(exec).not.toHaveBeenCalled() }) - test('calls the expected Shopify binary', async () => { + test('calls the current Shopify entry point with the canonical Node executable', async () => { // Given / When fetchNotificationsInBackground('theme:list', ['/path/to/node', '/path/to/shopify', 'theme', 'list'], { SHOPIFY_UNIT_TEST: 'false', @@ -453,7 +453,7 @@ describe('fetchNotificationsInBackground', () => { // Then expect(exec).toHaveBeenCalledWith( - '/path/to/node', + process.execPath, ['/path/to/shopify', 'notifications', 'list', '--ignore-errors'], expect.anything(), ) diff --git a/packages/cli-kit/src/public/node/notifications-system.ts b/packages/cli-kit/src/public/node/notifications-system.ts index 04dcfeb738e..53926890763 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -21,6 +21,7 @@ const COMMANDS_TO_SKIP = [ 'theme:init', 'hydrogen:init', 'cache:clear', + 'send-analytics', ] function url(): string { @@ -197,10 +198,10 @@ export function fetchNotificationsInBackground( environment: NodeJS.ProcessEnv = process.env, ): void { if (skipNotifications(currentCommand, environment)) return - if (!argv[0] || !argv[1]) return + if (!argv[1]) return // Run the Shopify command the same way as the current execution - const nodeBinary = argv[0] + const nodeBinary = process.execPath const shopifyBinary = argv[1] const args = [shopifyBinary, 'notifications', 'list', '--ignore-errors'] diff --git a/packages/cli-kit/src/public/node/system.test.ts b/packages/cli-kit/src/public/node/system.test.ts index 15c024fedaf..5feb4a15841 100644 --- a/packages/cli-kit/src/public/node/system.test.ts +++ b/packages/cli-kit/src/public/node/system.test.ts @@ -276,6 +276,28 @@ describe('execCommand', () => { expect(execa).toHaveBeenCalledWith('cat', [], expect.objectContaining({stdin: 'inherit'})) }) + test.skipIf(process.platform === 'win32')( + 'pipes input to a background process while ignoring its output', + async () => { + // Given + vi.mocked(which.sync).mockReturnValueOnce('/system/cat') + const unref = vi.fn() + const childProcess = Object.assign(Promise.resolve({}), {unref}) + vi.mocked(execa).mockReturnValueOnce(childProcess as any) + + // When + await system.exec('cat', [], {background: true, input: 'payload'}) + + // Then + expect(execa).toHaveBeenCalledWith( + 'cat', + [], + expect.objectContaining({input: 'payload', stdio: ['pipe', 'ignore', 'ignore']}), + ) + expect(unref).toHaveBeenCalledOnce() + }, + ) + test('raises an error if the command to run is found in the current directory', async () => { // Given vi.mocked(which.sync).mockReturnValueOnce('/currentDirectory/command') @@ -308,7 +330,11 @@ describe('execCommand', () => { describe('isStdinPiped', () => { test('returns true when stdin is a FIFO (pipe)', () => { // Given - vi.mocked(fs.fstatSync).mockReturnValue({isFIFO: () => true, isFile: () => false} as fs.Stats) + vi.mocked(fs.fstatSync).mockReturnValue({ + isFIFO: () => true, + isFile: () => false, + isSocket: () => false, + } as fs.Stats) // When const got = system.isStdinPiped() @@ -319,7 +345,26 @@ describe('isStdinPiped', () => { test('returns true when stdin is a file redirect', () => { // Given - vi.mocked(fs.fstatSync).mockReturnValue({isFIFO: () => false, isFile: () => true} as fs.Stats) + vi.mocked(fs.fstatSync).mockReturnValue({ + isFIFO: () => false, + isFile: () => true, + isSocket: () => false, + } as fs.Stats) + + // When + const got = system.isStdinPiped() + + // Then + expect(got).toBe(true) + }) + + test('returns true when stdin is a child-process pipe represented as a socket', () => { + // Given + vi.mocked(fs.fstatSync).mockReturnValue({ + isFIFO: () => false, + isFile: () => false, + isSocket: () => true, + } as fs.Stats) // When const got = system.isStdinPiped() @@ -330,7 +375,11 @@ describe('isStdinPiped', () => { test('returns false when stdin is a TTY (interactive)', () => { // Given - vi.mocked(fs.fstatSync).mockReturnValue({isFIFO: () => false, isFile: () => false} as fs.Stats) + vi.mocked(fs.fstatSync).mockReturnValue({ + isFIFO: () => false, + isFile: () => false, + isSocket: () => false, + } as fs.Stats) // When const got = system.isStdinPiped() diff --git a/packages/cli-kit/src/public/node/system.ts b/packages/cli-kit/src/public/node/system.ts index b3fb1441334..4cc5c5384e4 100644 --- a/packages/cli-kit/src/public/node/system.ts +++ b/packages/cli-kit/src/public/node/system.ts @@ -278,11 +278,12 @@ function buildExec( } const executionCwd = options?.cwd ?? cwd() checkCommandSafety(command, {cwd: executionCwd}) + const backgroundStdio = options?.input === undefined ? 'ignore' : (['pipe', 'ignore', 'ignore'] as const) const commandProcess = execa(command, args, { env, cwd: executionCwd, input: options?.input, - stdio: options?.background ? 'ignore' : options?.stdio, + stdio: options?.background ? backgroundStdio : options?.stdio, stdin: options?.stdin, stdout: options?.stdout === 'inherit' ? 'inherit' : undefined, stderr: options?.stderr === 'inherit' ? 'inherit' : undefined, @@ -387,7 +388,12 @@ export async function isWsl(overrides: WslDetectionOverrides = {}): Promise, diff --git a/packages/cli/oclif.manifest.json b/packages/cli/oclif.manifest.json index 98c1bcf5ac5..45ea376b354 100644 --- a/packages/cli/oclif.manifest.json +++ b/packages/cli/oclif.manifest.json @@ -6314,6 +6314,24 @@ "strict": true, "usage": "search [query]" }, + "send-analytics": { + "aliases": [ + ], + "args": { + }, + "enableJsonFlag": false, + "flags": { + }, + "hasDynamicHelp": false, + "hidden": true, + "hiddenAliases": [ + ], + "id": "send-analytics", + "pluginAlias": "@shopify/cli", + "pluginName": "@shopify/cli", + "pluginType": "core", + "strict": true + }, "store:auth": { "aliases": [ ], diff --git a/packages/cli/src/cli/commands/send-analytics.ts b/packages/cli/src/cli/commands/send-analytics.ts new file mode 100644 index 00000000000..d60db23b683 --- /dev/null +++ b/packages/cli/src/cli/commands/send-analytics.ts @@ -0,0 +1,10 @@ +import Command from '@shopify/cli-kit/node/base-command' +import {sendAnalyticsEventFromStdin} from '@shopify/cli-kit/node/analytics' + +export default class SendAnalytics extends Command { + static hidden = true + + async run(): Promise { + await sendAnalyticsEventFromStdin() + } +} diff --git a/packages/cli/src/index.ts b/packages/cli/src/index.ts index 7651e66d514..027585eac79 100644 --- a/packages/cli/src/index.ts +++ b/packages/cli/src/index.ts @@ -1,6 +1,7 @@ import VersionCommand from './cli/commands/version.js' import Search from './cli/commands/search.js' import Upgrade from './cli/commands/upgrade.js' +import SendAnalytics from './cli/commands/send-analytics.js' import Logout from './cli/commands/auth/logout.js' import Login from './cli/commands/auth/login.js' import CommandFlags from './cli/commands/debug/command-flags.js' @@ -149,6 +150,7 @@ export const COMMANDS: any = { search: Search, upgrade: Upgrade, version: VersionCommand, + 'send-analytics': SendAnalytics, help: HelpCommand, 'auth:logout': Logout, 'auth:login': Login, diff --git a/packages/theme/src/cli/commands/theme/dev.ts b/packages/theme/src/cli/commands/theme/dev.ts index 88979191f54..b765e11c0ca 100644 --- a/packages/theme/src/cli/commands/theme/dev.ts +++ b/packages/theme/src/cli/commands/theme/dev.ts @@ -17,6 +17,10 @@ import type {ErrorOverlayMode, LiveReload, ReconciliationStrategy} from '../../u type DevFlags = InferredFlags export default class Dev extends ThemeCommand { + public static get requiresSyncAnalytics(): boolean { + return true + } + static summary = 'Uploads the current theme as a development theme to the connected store, then prints theme editor and preview URLs to your terminal. While running, changes will push to the store in real time.'