diff --git a/packages/cli-kit/src/private/node/notifications-schema.ts b/packages/cli-kit/src/private/node/notifications-schema.ts new file mode 100644 index 00000000000..43d1dc79fb9 --- /dev/null +++ b/packages/cli-kit/src/private/node/notifications-schema.ts @@ -0,0 +1,28 @@ +import {zod} from '../../public/node/schema.js' + +export const NotificationSchema = zod.object({ + id: zod.string(), + message: zod.string(), + type: zod.enum(['info', 'warning', 'error']), + frequency: zod.enum(['always', 'once', 'once_a_day', 'once_a_week']), + ownerChannel: zod.string(), + cta: zod + .object({ + label: zod.string(), + url: zod.string().url(), + }) + .optional(), + title: zod.string().optional(), + minVersion: zod.string().optional(), + maxVersion: zod.string().optional(), + minDate: zod.string().optional(), + maxDate: zod.string().optional(), + commands: zod.array(zod.string()).optional(), + surface: zod.string().optional(), +}) + +export type Notification = zod.infer + +export const NotificationsSchema = zod.object({notifications: zod.array(NotificationSchema)}) + +export type Notifications = zod.infer diff --git a/packages/cli-kit/src/private/node/session-alias.ts b/packages/cli-kit/src/private/node/session-alias.ts new file mode 100644 index 00000000000..c0d93267756 --- /dev/null +++ b/packages/cli-kit/src/private/node/session-alias.ts @@ -0,0 +1,35 @@ +import {setCommandSessionId} from './session/command-session.js' +import * as sessionStore from './session/store.js' +import {AbortError} from '../../public/node/error.js' +import {outputContent, outputToken} from '../../public/node/output.js' + +/** + * Finds a stored Shopify account session by alias without changing the current session. + * + * @param alias - The account alias to find. + * @returns The matching session ID, or undefined if no session matches. + */ +export async function findSessionIdByAlias(alias: string): Promise { + return sessionStore.findSessionByAlias(alias) +} + +/** + * Selects a stored Shopify account session by alias for the current command process. + * + * @param alias - The account alias to select. Passing undefined clears the command selection. + */ +export async function setCurrentSessionAlias(alias?: string): Promise { + if (!alias) { + setCommandSessionId(undefined) + return + } + + const sessionId = await findSessionIdByAlias(alias) + if (!sessionId) { + throw new AbortError( + outputContent`No authenticated account found for alias ${outputToken.yellow(alias)}.`, + outputContent`Run ${outputToken.genericShellCommand(`shopify auth login`)} first.`, + ) + } + setCommandSessionId(sessionId) +} diff --git a/packages/cli-kit/src/private/node/session.ts b/packages/cli-kit/src/private/node/session.ts index 3eb9f9e5ee6..6a5ca5ac5e1 100644 --- a/packages/cli-kit/src/private/node/session.ts +++ b/packages/cli-kit/src/private/node/session.ts @@ -15,6 +15,7 @@ import {pollForDeviceAuthorization, requestDeviceAuthorization} from './session/ import {isThemeAccessSession} from './api/rest.js' import {getCurrentSessionId, setCurrentSessionId} from './conf-store.js' import {UserEmailQueryString, UserEmailQuery} from './api/graphql/business-platform-destinations/user-email.js' +import {getCommandSessionId} from './session/command-session.js' import {outputContent, outputToken, outputDebug, outputCompleted} from '../../public/node/output.js' import {themeToken} from '../../public/node/context/local.js' import {AbortError} from '../../public/node/error.js' @@ -25,6 +26,8 @@ import {nonRandomUUID} from '../../public/node/crypto.js' import {isEmpty} from '../../public/common/object.js' import {businessPlatformRequest} from '../../public/node/api/business-platform.js' +export {setCommandSessionId} from './session/command-session.js' + /** * Fetches the user's email from the Business Platform API * @param businessPlatformToken - The business platform token @@ -118,7 +121,6 @@ type AuthMethod = 'partners_token' | 'device_auth' | 'theme_access_token' | 'cus let userId: undefined | string let authMethod: AuthMethod = 'none' -let commandSessionId: string | undefined /** * Retrieves a stable user identifier for analytics, or `'unknown'` if none applies. @@ -180,10 +182,6 @@ export function setLastSeenAuthMethod(method: AuthMethod) { authMethod = method } -export function setCommandSessionId(sessionId: string | undefined) { - commandSessionId = sessionId -} - export interface EnsureAuthenticatedAdditionalOptions { noPrompt?: boolean forceRefresh?: boolean @@ -215,6 +213,7 @@ export async function ensureAuthenticated( const sessions = (await sessionStore.fetch()) ?? {} + const commandSessionId = getCommandSessionId() let currentSessionId = forceNewSession ? undefined : (commandSessionId ?? getCurrentSessionId()) if (!currentSessionId && !commandSessionId) { const userIds = Object.keys(sessions[fqdn] ?? {}) diff --git a/packages/cli-kit/src/private/node/session/command-session.ts b/packages/cli-kit/src/private/node/session/command-session.ts new file mode 100644 index 00000000000..e00302d3ec2 --- /dev/null +++ b/packages/cli-kit/src/private/node/session/command-session.ts @@ -0,0 +1,9 @@ +let commandSessionId: string | undefined + +export function getCommandSessionId(): string | undefined { + return commandSessionId +} + +export function setCommandSessionId(sessionId: string | undefined): void { + commandSessionId = sessionId +} diff --git a/packages/cli-kit/src/private/node/terminal.ts b/packages/cli-kit/src/private/node/terminal.ts new file mode 100644 index 00000000000..aa31ec87255 --- /dev/null +++ b/packages/cli-kit/src/private/node/terminal.ts @@ -0,0 +1,13 @@ +import {isTruthy} from '../../public/node/context/utilities.js' + +/** + * Check if the standard input and output streams support prompting. + * + * @returns True if the standard input and output streams support prompting. + */ +export function terminalSupportsPrompting(): boolean { + if (isTruthy(process.env.CI)) { + return false + } + return Boolean(process.stdin.isTTY && process.stdout.isTTY) +} diff --git a/packages/cli-kit/src/public/node/base-command.ts b/packages/cli-kit/src/public/node/base-command.ts index c81438e52ff..0d8436cc524 100644 --- a/packages/cli-kit/src/public/node/base-command.ts +++ b/packages/cli-kit/src/public/node/base-command.ts @@ -1,12 +1,11 @@ -import {isDevelopment} from './context/local.js' import {addPublicMetadata} from './metadata.js' import {AbortError} from './error.js' import {outputContent, outputResult, outputToken} from './output.js' -import {setCurrentSessionAlias} from './session.js' -import {terminalSupportsPrompting} from './system.js' import {hashString} from './crypto.js' import {isTruthy} from './context/utilities.js' import {setCurrentCommandId} from './global-context.js' +import {setCurrentSessionAlias} from '../../private/node/session-alias.js' +import {terminalSupportsPrompting} from '../../private/node/terminal.js' import {JsonMap} from '../../private/common/json.js' import {underscore} from '../common/string.js' import {Command, Config, Errors} from '@oclif/core' @@ -61,11 +60,6 @@ abstract class BaseCommand extends Command { protected async init(): Promise { this.exitWithTimestampWhenEnvVariablePresent() setCurrentCommandId(this.id ?? '') - if (!isDevelopment()) { - // This function runs just prior to `run` - const {registerCleanBugsnagErrorsFromWithinPlugins} = await import('./error-handler.js') - await registerCleanBugsnagErrorsFromWithinPlugins(this.config) - } await removeDuplicatedPlugins(this.config) this.showNpmFlagWarning() const {showNotificationsIfNeeded} = await import('./notifications-system.js') diff --git a/packages/cli-kit/src/public/node/error-handler.test.ts b/packages/cli-kit/src/public/node/error-handler.test.ts index 6a6e5a1a14e..bbd0c785d05 100644 --- a/packages/cli-kit/src/public/node/error-handler.test.ts +++ b/packages/cli-kit/src/public/node/error-handler.test.ts @@ -13,6 +13,7 @@ import {beforeEach, describe, expect, test, vi} from 'vitest' const onNotify = vi.fn() const capturedEventHandler = vi.fn() +const addOnError = vi.hoisted(() => vi.fn()) let lastBugsnagEvent: {addMetadata: ReturnType; groupingHash?: string} | undefined vi.mock('process') @@ -34,7 +35,7 @@ vi.mock('@bugsnag/js', () => { callback(null) }, isStarted: () => true, - addOnError: vi.fn(), + addOnError, }, } }) @@ -284,6 +285,18 @@ describe('sends errors to Bugsnag', () => { expect(mockEvent.setUser).toHaveBeenCalledWith(undefined) }) + test('registers plugin stack cleanup once before reporting errors', async () => { + const config = { + plugins: [], + runHook: vi.fn().mockResolvedValue({successes: []}), + } as unknown as NonNullable[2]> + + await sendErrorToBugsnag(new Error('first error'), 'unexpected_error', config) + await sendErrorToBugsnag(new Error('second error'), 'unexpected_error', config) + + expect(addOnError).toHaveBeenCalledOnce() + }) + test('attaches custom metadata with allowed slice_name when startCommand is present', async () => { await metadata.addSensitiveMetadata(() => ({ commandStartOptions: {startTime: Date.now(), startCommand: 'app dev', startArgs: []}, diff --git a/packages/cli-kit/src/public/node/error-handler.ts b/packages/cli-kit/src/public/node/error-handler.ts index 706249ed1d4..a11215520c4 100644 --- a/packages/cli-kit/src/public/node/error-handler.ts +++ b/packages/cli-kit/src/public/node/error-handler.ts @@ -11,6 +11,7 @@ import { cleanSingleStackTracePath, } from './error.js' import {outputDebug, outputInfo} from './output.js' +import {isDevelopment} from './context/local.js' import {getEnvironmentData} from '../../private/node/analytics.js' import {resolveErrorGrouping} from '../../private/node/analytics/error-grouping.js' import {isLocalEnvironment} from '../../private/node/context/service.js' @@ -66,7 +67,7 @@ const reportError = async (error: unknown, config?: Interfaces.Config): Promise< // Log an analytics event when there's an error await reportAnalyticsEvent({config, errorMessage: error instanceof Error ? error.message : undefined, exitMode}) } - await sendErrorToBugsnag(error, exitMode) + await sendErrorToBugsnag(error, exitMode, config) } /** @@ -77,6 +78,7 @@ const reportError = async (error: unknown, config?: Interfaces.Config): Promise< export async function sendErrorToBugsnag( error: unknown, exitMode: Omit, + config?: Interfaces.Config, ): Promise<{reported: false; error: unknown; unhandled: unknown} | {error: Error; reported: true; unhandled: boolean}> { try { if (isLocalEnvironment() || settings.debug) { @@ -146,6 +148,7 @@ export async function sendErrorToBugsnag( // Observe will use the IP when undefined userId = undefined } + if (config && !isDevelopment()) await registerCleanBugsnagErrorsFromWithinPlugins(config) await new Promise((resolve, reject) => { outputDebug(`Reporting ${unhandled ? 'unhandled' : 'handled'} error to Bugsnag: ${reportableError.message}`) const eventHandler = (event: Event) => { @@ -233,7 +236,14 @@ export function cleanStackFrameFilePath({ * Register a Bugsnag error listener to clean up stack traces for errors within plugin code. * */ +let pluginStackCleanupRegistration: Promise | undefined + export async function registerCleanBugsnagErrorsFromWithinPlugins(config: Interfaces.Config): Promise { + pluginStackCleanupRegistration ??= registerPluginStackCleanup(config) + await pluginStackCleanupRegistration +} + +async function registerPluginStackCleanup(config: Interfaces.Config): Promise { // Bugsnag have their own plug-ins that use this private field // eslint-disable-next-line @typescript-eslint/no-explicit-any 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 1eefbe57a37..1a9be2e3cad 100644 --- a/packages/cli-kit/src/public/node/notifications-system.test.ts +++ b/packages/cli-kit/src/public/node/notifications-system.test.ts @@ -449,10 +449,12 @@ describe('fetchNotificationsInBackground', () => { }) // Then - expect(exec).toHaveBeenCalledWith( - '/path/to/node', - ['/path/to/shopify', 'notifications', 'list', '--ignore-errors'], - expect.anything(), - ) + await vi.waitFor(() => { + expect(exec).toHaveBeenCalledWith( + '/path/to/node', + ['/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 d9eade1f1cb..26f314f81b5 100644 --- a/packages/cli-kit/src/public/node/notifications-system.ts +++ b/packages/cli-kit/src/public/node/notifications-system.ts @@ -1,15 +1,14 @@ import {versionSatisfies} from './node-package-manager.js' -import {renderError, renderInfo, renderWarning} from './ui.js' import {getCurrentCommandId} from './global-context.js' import {outputDebug} from './output.js' -import {zod} from './schema.js' import {AbortSilentError} from './error.js' import {isTruthy} from './context/utilities.js' -import {exec} from './system.js' import {jsonOutputEnabled} from './environment.js' -import {fetch} from './http.js' import {CLI_KIT_VERSION} from '../common/version.js' import {NotificationKey, NotificationsKey, cacheRetrieve, cacheStore} from '../../private/node/conf-store.js' +import type {Notification, Notifications} from '../../private/node/notifications-schema.js' + +export type {Notification, Notifications} from '../../private/node/notifications-schema.js' const URL = 'https://cdn.shopify.com/static/cli/notifications.json' const EMPTY_CACHE_MESSAGE = 'Cache is empty' @@ -27,31 +26,6 @@ function url(): string { return process.env.SHOPIFY_CLI_NOTIFICATIONS_URL ?? URL } -const NotificationSchema = zod.object({ - id: zod.string(), - message: zod.string(), - type: zod.enum(['info', 'warning', 'error']), - frequency: zod.enum(['always', 'once', 'once_a_day', 'once_a_week']), - ownerChannel: zod.string(), - cta: zod - .object({ - label: zod.string(), - url: zod.string().url(), - }) - .optional(), - title: zod.string().optional(), - minVersion: zod.string().optional(), - maxVersion: zod.string().optional(), - minDate: zod.string().optional(), - maxDate: zod.string().optional(), - commands: zod.array(zod.string()).optional(), - surface: zod.string().optional(), -}) -export type Notification = zod.infer - -const NotificationsSchema = zod.object({notifications: zod.array(NotificationSchema)}) -export type Notifications = zod.infer - /** * Shows notifications to the user if they meet the criteria specified in the notifications.json file. * @@ -98,7 +72,11 @@ function skipNotifications(currentCommand: string, environment: NodeJS.ProcessEn * @param notifications - The notifications to render. */ async function renderNotifications(notifications: Notification[]) { - notifications.slice(0, 2).forEach((notification) => { + const notificationsToRender = notifications.slice(0, 2) + if (notificationsToRender.length === 0) return + + const {renderError, renderInfo, renderWarning} = await import('./ui.js') + notificationsToRender.forEach((notification) => { const content = { headline: notification.title, body: notification.message.replace(/\\n/g, '\n'), @@ -132,6 +110,7 @@ export async function getNotifications(): Promise { const rawNotifications = cacheRetrieve(cacheKey)?.value as unknown as string if (!rawNotifications) throw new Error(EMPTY_CACHE_MESSAGE) const notifications: object = JSON.parse(rawNotifications) + const {NotificationsSchema} = await import('../../private/node/notifications-schema.js') return NotificationsSchema.parse(notifications) } @@ -142,6 +121,10 @@ export async function getNotifications(): Promise { */ export async function fetchNotifications(): Promise { outputDebug(`Fetching notifications...`) + const [{fetch}, {NotificationsSchema}] = await Promise.all([ + import('./http.js'), + import('../../private/node/notifications-schema.js'), + ]) const response = await fetch(url(), undefined, { useNetworkLevelRetry: false, useAbortSignal: true, @@ -187,13 +170,19 @@ export function fetchNotificationsInBackground( const args = [shopifyBinary, 'notifications', 'list', '--ignore-errors'] // eslint-disable-next-line no-void - void exec(nodeBinary, args, { - background: true, - env: {...process.env, SHOPIFY_CLI_NO_ANALYTICS: '1'}, - externalErrorHandler: async (error: unknown) => { + void import('./system.js') + .then(({exec}) => + exec(nodeBinary, args, { + background: true, + env: {...process.env, SHOPIFY_CLI_NO_ANALYTICS: '1'}, + externalErrorHandler: async (error: unknown) => { + outputDebug(`Failed to fetch notifications in background: ${(error as Error).message}`) + }, + }), + ) + .catch((error: unknown) => { outputDebug(`Failed to fetch notifications in background: ${(error as Error).message}`) - }, - }) + }) } /** diff --git a/packages/cli-kit/src/public/node/session.test.ts b/packages/cli-kit/src/public/node/session.test.ts index 449992bb4b8..3d7cfd5a7da 100644 --- a/packages/cli-kit/src/public/node/session.test.ts +++ b/packages/cli-kit/src/public/node/session.test.ts @@ -14,12 +14,8 @@ import { import {nonRandomUUID} from './crypto.js' import {getAppAutomationToken} from './environment.js' import {shopifyFetch} from './http.js' -import { - ensureAuthenticated, - setCommandSessionId, - setLastSeenAuthMethod, - setLastSeenUserIdAfterAuth, -} from '../../private/node/session.js' +import {ensureAuthenticated, setLastSeenAuthMethod, setLastSeenUserIdAfterAuth} from '../../private/node/session.js' +import {setCommandSessionId} from '../../private/node/session/command-session.js' import * as sessionStore from '../../private/node/session/store.js' import {ApplicationToken} from '../../private/node/session/schema.js' import { @@ -39,6 +35,7 @@ const partnersToken: ApplicationToken = { } vi.mock('../../private/node/session.js') +vi.mock('../../private/node/session/command-session.js') vi.mock('../../private/node/session/exchange.js') vi.mock('../../private/node/session/store.js') vi.mock('./environment.js') diff --git a/packages/cli-kit/src/public/node/session.ts b/packages/cli-kit/src/public/node/session.ts index 15be5d39cbf..79812ce4e03 100644 --- a/packages/cli-kit/src/public/node/session.ts +++ b/packages/cli-kit/src/public/node/session.ts @@ -17,12 +17,13 @@ import { PartnersAPIScope, StorefrontRendererScope, ensureAuthenticated, - setCommandSessionId, setLastSeenAuthMethod, setLastSeenUserIdAfterAuth, } from '../../private/node/session.js' import {isThemeAccessSession} from '../../private/node/api/rest.js' +export {findSessionIdByAlias, setCurrentSessionAlias} from '../../private/node/session-alias.js' + /** * Session Object to access the Admin API, includes the token and the store FQDN. */ @@ -52,37 +53,6 @@ export function setLastSeenUserId(userId: string): void { setLastSeenUserIdAfterAuth(userId) } -/** - * Finds a stored Shopify account session by alias without changing the current session. - * - * @param alias - The account alias to find. - * @returns The matching session ID, or undefined if no session matches. - */ -export async function findSessionIdByAlias(alias: string): Promise { - return sessionStore.findSessionByAlias(alias) -} - -/** - * Selects a stored Shopify account session by alias for the current command process. - * - * @param alias - The account alias to select. Passing undefined clears the command selection. - */ -export async function setCurrentSessionAlias(alias?: string): Promise { - if (!alias) { - setCommandSessionId(undefined) - return - } - - const sessionId = await findSessionIdByAlias(alias) - if (!sessionId) { - throw new AbortError( - outputContent`No authenticated account found for alias ${outputToken.yellow(alias)}.`, - outputContent`Run ${outputToken.genericShellCommand(`shopify auth login`)} first.`, - ) - } - setCommandSessionId(sessionId) -} - interface UserAccountInfo { type: 'UserAccount' email: string diff --git a/packages/cli-kit/src/public/node/system.ts b/packages/cli-kit/src/public/node/system.ts index aa997f8c5d8..e8d25056c9b 100644 --- a/packages/cli-kit/src/public/node/system.ts +++ b/packages/cli-kit/src/public/node/system.ts @@ -16,6 +16,8 @@ import {fstatSync} from 'fs' import {setTimeout} from 'timers/promises' import type {Writable, Readable} from 'stream' +export {terminalSupportsPrompting} from '../../private/node/terminal.js' + /** * The maximum size of data that can be read from stdin in bytes. * This is to prevent memory exhaustion when reading from stdin. @@ -332,18 +334,6 @@ export function terminalSupportsHyperlinks(): boolean { return supportsHyperlinks.stdout } -/** - * Check if the standard input and output streams support prompting. - * - * @returns True if the standard input and output streams support prompting. - */ -export function terminalSupportsPrompting(): boolean { - if (isTruthy(process.env.CI)) { - return false - } - return Boolean(process.stdin.isTTY && process.stdout.isTTY) -} - /** * Check if the current environment is a CI environment. *