Skip to content

Commit

Permalink
Handle dev-session errors for each extension (#4864)
Browse files Browse the repository at this point in the history
<!--
  ☝️How to write a good PR title:
  - Prefix it with [Feature] (if applicable)
  - Start with a verb, for example: Add, Delete, Improve, Fix…
  - Give as much context as necessary and as little as possible
  - Use a draft PR while it’s a work in progress
-->

### WHY are these changes introduced?

Fixes #0000 <!-- link to issue if one exists -->

<!--
  Context about the problem that’s being addressed.
-->

### WHAT is this pull request doing?

<!--
  Summary of the changes committed.
  Before / after screenshots appreciated for UI changes.
-->

### How to test your changes?

<!--
  Please, provide steps for the reviewer to test your changes locally.
-->

### Post-release steps

<!--
  If changes require post-release steps, for example merging and publishing some documentation changes,
  specify it in this section and add the label "includes-post-release-steps".
  If it doesn't, feel free to remove this section.
-->

### Measuring impact

How do we know this change was effective? Please choose one:

- [ ] n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
- [ ] Existing analytics will cater for this addition
- [ ] PR includes analytics changes to measure impact

### Checklist

- [ ] I've considered possible cross-platform impacts (Mac, Linux, Windows)
- [ ] I've considered possible [documentation](https://shopify.dev) changes
  • Loading branch information
isaacroldan authored Nov 22, 2024
2 parents 7df6667 + 5f6df1a commit 2a139a2
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 42 deletions.
85 changes: 55 additions & 30 deletions packages/app/src/cli/services/dev/processes/dev-session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ import {outputContent, outputDebug, outputToken} from '@shopify/cli-kit/node/out
import {endHRTimeInMs, startHRTime} from '@shopify/cli-kit/node/hrtime'
import {performActionWithRetryAfterRecovery} from '@shopify/cli-kit/common/retry'
import {useConcurrentOutputContext} from '@shopify/cli-kit/node/ui/components'
import {JsonMapType} from '@shopify/cli-kit/node/toml'
import {AbortError} from '@shopify/cli-kit/node/error'
import {Writable} from 'stream'

interface DevSessionOptions {
Expand All @@ -38,11 +40,18 @@ export interface DevSessionProcess extends BaseProcess<DevSessionOptions> {
type: 'dev-session'
}

interface DevSessionResult {
status: 'updated' | 'created' | 'aborted' | 'error'
error?: string
interface UserError {
message: string
on: JsonMapType
field?: string[] | null
category: string
}

type DevSessionResult =
| {status: 'updated' | 'created' | 'aborted'}
| {status: 'remote-error'; error: UserError[]}
| {status: 'unknown-error'; error: Error}

let bundleControllers: AbortController[] = []

// Current status of the dev session
Expand Down Expand Up @@ -109,7 +118,7 @@ export const pushUpdatesForDevSession: DevProcessFunction<DevSessionOptions> = a
const networkStartTime = startHRTime()
await performActionWithRetryAfterRecovery(async () => {
const result = await bundleExtensionsAndUpload({...processOptions, app: event.app})
await handleDevSessionResult(result, processOptions, event)
await handleDevSessionResult(result, {...processOptions, app: event.app}, event)
const endTime = endHRTimeInMs(event.startTime)
const endNetworkTime = endHRTimeInMs(networkStartTime)
outputDebug(`✅ Event handled [Network: ${endNetworkTime}ms -- Total: ${endTime}ms]`, processOptions.stdout)
Expand All @@ -118,22 +127,11 @@ export const pushUpdatesForDevSession: DevProcessFunction<DevSessionOptions> = a
.onStart(async (event) => {
await performActionWithRetryAfterRecovery(async () => {
const result = await bundleExtensionsAndUpload({...processOptions, app: event.app})
await handleDevSessionResult(result, processOptions)
await handleDevSessionResult(result, {...processOptions, app: event.app})
}, refreshToken)
})
}

// We shouldn't need this, as the dev session create mutation shouldn't hang, but it can be a temporary
// utility to debug issues with the dev API.
function startTimeout(processOptions: DevSessionProcessOptions) {
setTimeout(() => {
if (!isDevSessionReady) {
printError('❌ Timeout, session failed to start in 30s, please try again.', processOptions.stdout).catch(() => {})
process.exit(1)
}
}, 30000)
}

async function handleDevSessionResult(
result: DevSessionResult,
processOptions: DevSessionProcessOptions,
Expand All @@ -149,13 +147,16 @@ async function handleDevSessionResult(
await printWarning(message.value, processOptions.stdout)
}
} else if (result.status === 'created') {
isDevSessionReady = true
await printSuccess(`✅ Ready, watching for changes in your app `, processOptions.stdout)
} else if (result.status === 'aborted') {
outputDebug('❌ Session update aborted (new change detected)', processOptions.stdout)
} else {
await printError(`❌ Error`, processOptions.stderr)
await printError(`└ ${result.error}`, processOptions.stderr)
outputDebug('❌ Session update aborted (new change detected or error in Session Update)', processOptions.stdout)
} else if (result.status === 'remote-error' || result.status === 'unknown-error') {
await processUserErrors(result.error, processOptions, processOptions.stdout)
}

// If we failed to create a session, exit the process
if (!isDevSessionReady) throw new AbortError('Failed to create dev session')
}

/**
Expand Down Expand Up @@ -213,13 +214,14 @@ async function bundleExtensionsAndUpload(options: DevSessionProcessOptions): Pro
if (currentBundleController.signal.aborted) return {status: 'aborted'}
try {
if (isDevSessionReady) {
await options.developerPlatformClient.devSessionUpdate(payload)
const result = await options.developerPlatformClient.devSessionUpdate(payload)
const errors = result.devSessionUpdate?.userErrors ?? []
if (errors.length) return {status: 'remote-error', error: errors}
return {status: 'updated'}
} else {
startTimeout(options)
await options.developerPlatformClient.devSessionCreate(payload)
// eslint-disable-next-line require-atomic-updates
isDevSessionReady = true
const result = await options.developerPlatformClient.devSessionCreate(payload)
const errors = result.devSessionCreate?.userErrors ?? []
if (errors.length) return {status: 'remote-error', error: errors}
return {status: 'created'}
}
// eslint-disable-next-line @typescript-eslint/no-explicit-any
Expand All @@ -228,7 +230,27 @@ async function bundleExtensionsAndUpload(options: DevSessionProcessOptions): Pro
// Re-throw the error so the recovery procedure can be executed
throw new Error('Unauthorized')
} else {
return {status: 'error', error: error.message}
return {status: 'unknown-error', error}
}
}
}

async function processUserErrors(
errors: UserError[] | Error | string,
options: DevSessionProcessOptions,
stdout: Writable,
) {
if (typeof errors === 'string') {
await printError(errors, stdout)
} else if (errors instanceof Error) {
await printError(errors.message, stdout)
} else {
for (const error of errors) {
const on = error.on ? (error.on[0] as {user_identifier: unknown}) : undefined
// If we have information about the extension that caused the error, use the handle as prefix in the output.
const extension = options.app.allExtensions.find((ext) => ext.uid === on?.user_identifier)
// eslint-disable-next-line no-await-in-loop
await printError(error.message, stdout, extension?.handle ?? 'dev-session')
}
}
}
Expand All @@ -237,17 +259,20 @@ async function printWarning(message: string, stdout: Writable) {
await printLogMessage(outputContent`${outputToken.yellow(message)}`.value, stdout)
}

async function printError(message: string, stdout: Writable) {
await printLogMessage(outputContent`${outputToken.errorText(message)}`.value, stdout)
async function printError(message: string, stdout: Writable, prefix?: string) {
const header = outputToken.errorText(`❌ Error`)
const content = outputToken.errorText(`└ ${message}`)
await printLogMessage(outputContent`${header}`.value, stdout, prefix)
await printLogMessage(outputContent`${content}`.value, stdout, prefix)
}

async function printSuccess(message: string, stdout: Writable) {
await printLogMessage(outputContent`${outputToken.green(message)}`.value, stdout)
}

// Helper function to print to terminal using output context with stripAnsi disabled.
async function printLogMessage(message: string, stdout: Writable) {
await useConcurrentOutputContext({outputPrefix: 'dev-session', stripAnsi: false}, () => {
async function printLogMessage(message: string, stdout: Writable, prefix?: string) {
await useConcurrentOutputContext({outputPrefix: prefix ?? 'dev-session', stripAnsi: false}, () => {
stdout.write(message)
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -794,22 +794,12 @@ export class AppManagementClient implements DeveloperPlatformClient {

async devSessionCreate({appId, assetsUrl, shopFqdn}: DevSessionOptions): Promise<DevSessionCreateMutation> {
const appIdNumber = String(numberFromGid(appId))
const result = await appDevRequest(DevSessionCreate, shopFqdn, await this.token(), {appId: appIdNumber, assetsUrl})
if (result.devSessionCreate?.userErrors?.length) {
const error = result.devSessionCreate.userErrors.map((err) => err.message).join('\n')
throw new AbortError(error)
}
return result
return appDevRequest(DevSessionCreate, shopFqdn, await this.token(), {appId: appIdNumber, assetsUrl})
}

async devSessionUpdate({appId, assetsUrl, shopFqdn}: DevSessionOptions): Promise<DevSessionUpdateMutation> {
const appIdNumber = String(numberFromGid(appId))
const result = await appDevRequest(DevSessionUpdate, shopFqdn, await this.token(), {appId: appIdNumber, assetsUrl})
if (result.devSessionUpdate?.userErrors?.length) {
const error = result.devSessionUpdate.userErrors.map((err) => err.message).join('\n')
throw new AbortError(error)
}
return result
return appDevRequest(DevSessionUpdate, shopFqdn, await this.token(), {appId: appIdNumber, assetsUrl})
}

async devSessionDelete({appId, shopFqdn}: Omit<DevSessionOptions, 'assetsUrl'>): Promise<DevSessionDeleteMutation> {
Expand Down

0 comments on commit 2a139a2

Please sign in to comment.