Skip to content

On app selecetion keep retrying when failing but avoid capturing unrelated errors #6904

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
alfonso-noriega wants to merge 1 commit into
base: main
Choose a base branch
from
+19 −47
Open

On app selecetion keep retrying when failing but avoid capturing unrelated errors #6904

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 1 addition & 9 deletions packages/app/src/cli/prompts/dev.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,15 +64,7 @@ export async function selectAppPrompt(
},
})

const appChoice = currentAppChoices.find((app) => app.apiKey === apiKey)!

if (!appChoice) {
throw new Error(
`Unable to select an app: the selection prompt was interrupted multiple times./n
Api key ${apiKey} was selected but not found in ${currentAppChoices.map((app) => app.apiKey).join(', ')}`,
)
}
return appChoice
return currentAppChoices.find((app) => app.apiKey === apiKey)!
Copy link

@binks-code-reviewer binks-code-reviewer bot Feb 27, 2026

Choose a reason for hiding this comment

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

Non-null assertion can crash with unhelpful error (diagnostics regression)

selectAppPrompt now returns currentAppChoices.find((app) => app.apiKey === apiKey)!. If renderAutocompletePrompt returns a value not present in currentAppChoices (the incident described in the PR links), find(...) returns undefined and ! can cause a runtime failure with a generic error instead of a targeted, actionable message.

Impact:

  • User: shopify app dev can crash during selection with a confusing error, blocking development.
  • Support/diagnostics: reduced ability to diagnose prompt/list mismatch issues.
  • Scale: affects any environment where prompt selection can desync from the in-memory list.
  • Consequences: hard crash + loss of diagnostic context.

}

interface SelectStorePromptOptions {
Expand Down
56 changes: 18 additions & 38 deletions packages/app/src/cli/services/dev/select-app.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {getCachedCommandInfo, setCachedCommandTomlPreference} from '../local-sto
import {CreateAppOptions, DeveloperPlatformClient} from '../../utilities/developer-platform-client.js'
import {AppConfigurationFileName} from '../../models/app/loader.js'
import {BugError} from '@shopify/cli-kit/node/error'
import {outputInfo, outputDebug} from '@shopify/cli-kit/node/output'
import {outputInfo} from '@shopify/cli-kit/node/output'

const MAX_PROMPT_RETRIES = 2

Expand Down Expand Up @@ -51,52 +51,32 @@ export async function selectOrCreateApp(
const tomls = (cachedData?.tomls as {[key: string]: AppConfigurationFileName}) ?? {}

for (let attempt = 0; attempt < MAX_PROMPT_RETRIES; attempt++) {
try {
// eslint-disable-next-line no-await-in-loop
const app = await selectAppPrompt(
searchForAppsByNameFactory(developerPlatformClient, org.id),
apps,
hasMorePages,
{directory: options.directory},
)
// eslint-disable-next-line no-await-in-loop
const app = await selectAppPrompt(
searchForAppsByNameFactory(developerPlatformClient, org.id),
apps,
hasMorePages,
{directory: options.directory},
)
Copy link

@binks-code-reviewer binks-code-reviewer bot Feb 27, 2026

Choose a reason for hiding this comment

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

Retry behavior removed for prompt failures; loop no longer matches intent

The PR removes the try/catch around await selectAppPrompt(...) and around appFromIdentifiers. If selectAppPrompt throws (prompt interrupted/UI failure/etc.), the exception exits selectOrCreateApp immediately and no retry occurs, even though the loop is still present. This contradicts the PR intent (“keep retrying…”) and the previous behavior (retrying non-BugError failures). The old code already avoided retrying BugError; removing the catch stops retries on prompt errors entirely.

Impact:

  • User: transient prompt interruptions won’t be retried; immediate failure.
  • Infra/support: more repeated command invocations/tickets; less observability.
  • Scale: any user with flaky terminal/container/WSL interruptions.
  • Consequences: higher failure rate and worse UX.


const selectedToml = tomls[app.apiKey]
if (selectedToml) setCachedCommandTomlPreference(selectedToml)
const selectedToml = tomls[app.apiKey]
if (selectedToml) setCachedCommandTomlPreference(selectedToml)

// eslint-disable-next-line no-await-in-loop
const fullSelectedApp = await developerPlatformClient.appFromIdentifiers(app.apiKey)

if (!fullSelectedApp) {
throw new BugError(
`Unable to fetch app ${app.apiKey} from Shopify`,
'Try running `shopify app config link` to connect to an app you have access to.',
)
}
// eslint-disable-next-line no-await-in-loop
const fullSelectedApp = await developerPlatformClient.appFromIdentifiers(app.apiKey)

if (fullSelectedApp) {
return fullSelectedApp
} catch (error) {
// Don't retry BugError - those indicate actual bugs, not transient issues
if (error instanceof BugError) {
throw error
}

const errorObj = error as Error

// Log each attempt for observability
outputDebug(`App selection attempt ${attempt + 1}/${MAX_PROMPT_RETRIES} failed: ${errorObj.message}`)

// If we have retries left, inform user and retry
if (attempt < MAX_PROMPT_RETRIES - 1) {
outputInfo('App selection failed. Retrying...')
} else {
throw new BugError(errorObj.message, TRY_MESSAGE)
}
}
// If we have retries left, inform user and retry
if (attempt < MAX_PROMPT_RETRIES - 1) {
outputInfo('App selection failed. Retrying...')
}
}

// User-facing error message with key diagnostic info
const errorMessage = [
'Unable to select an app: the selection prompt was interrupted multiple times.',
'Unable to select an app: the selection prompt failed multiple times.',
'',
`Available apps: ${apps.length}`,
].join('\n')
Expand Down
Loading