Fetch notifications in background

.changeset/violet-carrots-argue.md

@@ -0,0 +1,5 @@
+---
+'@shopify/cli-kit': patch
+---
+
+Fetch notifications in background

packages/cli-kit/assets/fetch-notifications.js

@@ -0,0 +1,25 @@
+import fetch from 'node-fetch'
+import Conf from 'conf'
+
+async function fetchNotifications() {
+  try {
+    const url = process.env.SHOPIFY_CLI_NOTIFICATIONS_URL ?? 'https://cdn.shopify.com/static/cli/notifications.json'
+    const response = await fetch(url, {signal: AbortSignal.timeout(3 * 1000)})
+    if (response.status === 200) {
+      storeNotifications(url, await response.text())
+      console.log(`Notifications from ${url} cached successfully`)
+    }
+  } catch (error) {
+    const errorMessage = `Error fetching notifications: ${error.message}`
+    console.error(errorMessage)
+  }
+}
+
+function storeNotifications(url, notifications) {
+  const config = new Conf({projectName: 'shopify-cli-kit'})
+  const cache = config.get('cache') || {}
+  cache[`notifications-${url}`] = {value: notifications, timestamp: Date.now()}
+  config.set('cache', cache)
+}
+
+await fetchNotifications()

packages/cli-kit/src/public/node/hooks/postrun.ts

@@ -3,12 +3,14 @@ import {reportAnalyticsEvent} from '../analytics.js'
 import {outputDebug} from '../../../public/node/output.js'
 import BaseCommand from '../base-command.js'
 import * as metadata from '../../../public/node/metadata.js'
+import {exec} from '../system.js'
 import {Command, Hook} from '@oclif/core'
 
 // This hook is called after each successful command run. More info: https://oclif.io/docs/hooks
 export const hook: Hook.Postrun = async ({config, Command}) => {
   await detectStopCommand(Command as unknown as typeof Command)
   await reportAnalyticsEvent({config, exitMode: 'ok'})
+  await exec('node', [require.resolve('@shopify/cli-kit/assets/fetch-notifications.js')], {background: true})
   deprecationsHook(Command)
 
   const command = Command.id.replace(/:/g, ' ')
@@ -26,10 +28,10 @@ async function detectStopCommand(commandClass: Command.Class | typeof BaseComman
     const stopCommand = (commandClass as typeof BaseCommand).analyticsStopCommand()
     if (stopCommand) {
       const {commandStartOptions} = metadata.getAllSensitiveMetadata()
+      if (!commandStartOptions) return
       await metadata.addSensitiveMetadata(() => ({
         commandStartOptions: {
-          // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-          ...commandStartOptions!,
+          ...commandStartOptions,
           startTime: currentTime,
           startCommand: stopCommand,
         },

packages/cli-kit/src/public/node/notifications-system.test.ts

@@ -1,7 +1,7 @@
 import {Notification, filterNotifications, showNotificationsIfNeeded} from './notifications-system.js'
 import {renderError, renderInfo, renderWarning} from './ui.js'
 import {sniffForJson} from './path.js'
-import {cacheRetrieve, cacheRetrieveOrRepopulate} from '../../private/node/conf-store.js'
+import {cacheRetrieve} from '../../private/node/conf-store.js'
 import {afterEach, describe, expect, test, vi} from 'vitest'
 
 vi.mock('./ui.js')
@@ -333,7 +333,7 @@ describe('showNotificationsIfNeeded', () => {
   test('an info notification triggers a renderInfo call', async () => {
     // Given
     const notifications = [infoNotification]
-    vi.mocked(cacheRetrieveOrRepopulate).mockResolvedValue(JSON.stringify({notifications}))
+    vi.mocked(cacheRetrieve).mockReturnValue({value: JSON.stringify({notifications}), timestamp: 0})
 
     // When
     await showNotificationsIfNeeded(undefined, {SHOPIFY_UNIT_TEST: 'false'})
@@ -345,7 +345,7 @@ describe('showNotificationsIfNeeded', () => {
   test('a warning notification triggers a renderWarning call', async () => {
     // Given
     const notifications = [warningNotification]
-    vi.mocked(cacheRetrieveOrRepopulate).mockResolvedValue(JSON.stringify({notifications}))
+    vi.mocked(cacheRetrieve).mockReturnValue({value: JSON.stringify({notifications}), timestamp: 0})
 
     // When
     await showNotificationsIfNeeded(undefined, {SHOPIFY_UNIT_TEST: 'false'})
@@ -357,7 +357,7 @@ describe('showNotificationsIfNeeded', () => {
   test('an error notification triggers a renderError call and throws an error', async () => {
     // Given
     const notifications = [errorNotification]
-    vi.mocked(cacheRetrieveOrRepopulate).mockResolvedValue(JSON.stringify({notifications}))
+    vi.mocked(cacheRetrieve).mockReturnValue({value: JSON.stringify({notifications}), timestamp: 0})
 
     // When
     await expect(showNotificationsIfNeeded(undefined, {SHOPIFY_UNIT_TEST: 'false'})).rejects.toThrowError()
@@ -369,7 +369,7 @@ describe('showNotificationsIfNeeded', () => {
   test('notifications are skipped on CI', async () => {
     // Given
     const notifications = [infoNotification]
-    vi.mocked(cacheRetrieveOrRepopulate).mockResolvedValue(JSON.stringify({notifications}))
+    vi.mocked(cacheRetrieve).mockReturnValue({value: JSON.stringify({notifications}), timestamp: 0})
 
     // When
     await showNotificationsIfNeeded(undefined, {SHOPIFY_UNIT_TEST: 'false', CI: 'true'})
@@ -381,7 +381,7 @@ describe('showNotificationsIfNeeded', () => {
   test('notifications are skipped on tests', async () => {
     // Given
     const notifications = [infoNotification]
-    vi.mocked(cacheRetrieveOrRepopulate).mockResolvedValue(JSON.stringify({notifications}))
+    vi.mocked(cacheRetrieve).mockReturnValue({value: JSON.stringify({notifications}), timestamp: 0})
 
     // When
     await showNotificationsIfNeeded(undefined, {SHOPIFY_UNIT_TEST: 'true'})
@@ -393,7 +393,7 @@ describe('showNotificationsIfNeeded', () => {
   test('notifications are skipped when using --json flag', async () => {
     // Given
     const notifications = [infoNotification]
-    vi.mocked(cacheRetrieveOrRepopulate).mockResolvedValue(JSON.stringify({notifications}))
+    vi.mocked(cacheRetrieve).mockReturnValue({value: JSON.stringify({notifications}), timestamp: 0})
     vi.mocked(sniffForJson).mockReturnValue(true)
 
     // When
@@ -406,7 +406,7 @@ describe('showNotificationsIfNeeded', () => {
   test('notifications are skipped when using SHOPIFY_FLAG_JSON', async () => {
     // Given
     const notifications = [infoNotification]
-    vi.mocked(cacheRetrieveOrRepopulate).mockResolvedValue(JSON.stringify({notifications}))
+    vi.mocked(cacheRetrieve).mockReturnValue({value: JSON.stringify({notifications}), timestamp: 0})
 
     // When
     await showNotificationsIfNeeded(undefined, {SHOPIFY_UNIT_TEST: 'false', SHOPIFY_FLAG_JSON: 'true'})

packages/cli-kit/src/public/node/notifications-system.ts

@@ -7,19 +7,17 @@ import {AbortSilentError} from './error.js'
 import {isTruthy} from './context/utilities.js'
 import {jsonOutputEnabled} from './environment.js'
 import {CLI_KIT_VERSION} from '../common/version.js'
-import {
-  NotificationKey,
-  NotificationsKey,
-  cacheRetrieve,
-  cacheRetrieveOrRepopulate,
-  cacheStore,
-} from '../../private/node/conf-store.js'
-import {fetch} from '@shopify/cli-kit/node/http'
+import {NotificationKey, NotificationsKey, cacheRetrieve, cacheStore} from '../../private/node/conf-store.js'
 
 const URL = 'https://cdn.shopify.com/static/cli/notifications.json'
-const CACHE_DURATION_IN_MS = 3600 * 1000
+const EMPTY_CACHE_MESSAGE = 'Cache is empty'
 
-function url(): string {
+/**
+ * Returns the URL to retrieve the notifications.
+ *
+ * @returns - The value from SHOPIFY_CLI_NOTIFICATIONS_URL or the default URL (https://cdn.shopify.com/static/cli/notifications.json).
+ */
+export function notificationsUrl(): string {
   return process.env.SHOPIFY_CLI_NOTIFICATIONS_URL ?? URL
 }
 
@@ -72,6 +70,7 @@ export async function showNotificationsIfNeeded(
     if (error.message === 'abort') throw new AbortSilentError()
     const errorMessage = `Error retrieving notifications: ${error.message}`
     outputDebug(errorMessage)
+    if (error.message === EMPTY_CACHE_MESSAGE) return
     // This is very prone to becoming a circular dependency, so we import it dynamically
     const {sendErrorToBugsnag} = await import('./error-handler.js')
     await sendErrorToBugsnag(errorMessage, 'unexpected_error')
@@ -113,27 +112,18 @@ async function renderNotifications(notifications: Notification[]) {
 }
 
 /**
- * Get notifications list from cache (refreshed every hour) or fetch it if not present.
+ * Get notifications list from cache, that is updated in the background from the fetch-notifications.json script.
  *
  * @returns A Notifications object.
  */
 export async function getNotifications(): Promise<Notifications> {
-  const cacheKey: NotificationsKey = `notifications-${url()}`
-  const rawNotifications = await cacheRetrieveOrRepopulate(cacheKey, fetchNotifications, CACHE_DURATION_IN_MS)
+  const cacheKey: NotificationsKey = `notifications-${notificationsUrl()}`
+  const rawNotifications = cacheRetrieve(cacheKey)?.value as unknown as string
+  if (!rawNotifications) throw new Error(EMPTY_CACHE_MESSAGE)
   const notifications: object = JSON.parse(rawNotifications)
   return NotificationsSchema.parse(notifications)
 }
 
-/**
- * Fetch notifications from GitHub.
- */
-async function fetchNotifications(): Promise<string> {
-  outputDebug(`No cached notifications found. Fetching them...`)
-  const response = await fetch(url(), {signal: AbortSignal.timeout(3 * 1000)})
-  if (response.status !== 200) throw new Error(`Failed to fetch notifications: ${response.statusText}`)
-  return response.text() as unknown as string
-}
-
 /**
  * Filters notifications based on the version of the CLI.
  *

packages/cli-kit/src/public/node/system.ts

@@ -20,6 +20,7 @@ export interface ExecOptions {
   signal?: AbortSignal
   // Custom handler if process exits with a non-zero code
   externalErrorHandler?: (error: unknown) => Promise<void>
+  background?: boolean
 }
 
 /**
@@ -54,6 +55,11 @@ export async function captureOutput(command: string, args: string[], options?: E
  */
 export async function exec(command: string, args: string[], options?: ExecOptions): Promise<void> {
   const commandProcess = buildExec(command, args, options)
+
+  if (options?.background) {
+    commandProcess.unref()
+  }
+
   if (options?.stderr && options.stderr !== 'inherit') {
     commandProcess.stderr?.pipe(options.stderr, {end: false})
   }
@@ -104,16 +110,18 @@ function buildExec(command: string, args: string[], options?: ExecOptions): Exec
     env,
     cwd: options?.cwd,
     input: options?.input,
-    stdio: options?.stdio,
+    stdio: options?.background ? 'ignore' : options?.stdio,
     stdin: options?.stdin,
     stdout: options?.stdout === 'inherit' ? 'inherit' : undefined,
     stderr: options?.stderr === 'inherit' ? 'inherit' : undefined,
     // Setting this to false makes it possible to kill the main process
     // and all its sub-processes with Ctrl+C on Windows
     windowsHide: false,
+    detached: options?.background,
+    cleanup: !options?.background,
   })
   outputDebug(`
-Running system process:
+Running system process${options?.background ? ' in background' : ''}:
   · Command: ${command} ${args.join(' ')}
   · Working directory: ${options?.cwd ?? cwd()}
 `)