Skip to content

Commit 4b99cef

Browse files
committed
Stop reporting expected Admin API failures from store execute as CLI bugs
Two error shapes from the Admin API were being misclassified as `BugError` and reported to Observe (issue shop/issues#32996): - HTTP 402 "Unavailable Shop" \u2014 the shop is frozen / on hold; this is a store-state issue, not a CLI bug. - "The user aborted a request." \u2014 surfaces from node-fetch when an AbortController signal fires (CLI-side fetch timeout or user cancel). Both were getting wrapped by `fetchApiVersions`'s "unknown error" catch in cli-kit, which destroys the original error type. To classify them locally without reaching past the public API, `store execute` now drives its API-version lookup directly via `graphqlRequest` against an inline `publicApiVersions` query \u2014 the same primitive `admin-transport.ts` uses for the user's query. Both phases now see raw `ClientError` / `Error` shapes and share a single classifier in `admin-errors.ts`. Other commands that hit the Admin API on startup are unaffected; cli-kit is untouched. - `packages/store/.../execute/admin-errors.ts` (new): structural `ClientError` shape check, abort detection, and `classifyAdminApiError` that maps known non-bug shapes to user-facing `AbortError`s. - `packages/store/.../execute/admin-context.ts`: drives the version lookup via `graphqlRequest` directly. 401/404 still trigger the re-auth path (now via structural status check rather than parsing message strings); 402 and aborts go through the shared classifier. - `packages/store/.../execute/admin-transport.ts`: same shared classifier covers aborts on the execute-phase request. New `admin-errors.test.ts` covers the classifier; existing context and transport tests updated to mock `graphqlRequest` directly.
1 parent 2e14345 commit 4b99cef

4 files changed

Lines changed: 354 additions & 77 deletions

File tree

Lines changed: 28 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,16 @@
11
import {prepareAdminStoreGraphQLContext} from './admin-context.js'
2-
import {clearStoredStoreAppSession} from '../auth/session-store.js'
2+
import {fetchPublicApiVersions} from './admin-transport.js'
33
import {loadStoredStoreSession} from '../auth/session-lifecycle.js'
44
import {STORE_AUTH_APP_CLIENT_ID} from '../auth/config.js'
55
import {AbortError} from '@shopify/cli-kit/node/error'
6-
import {fetchApiVersions} from '@shopify/cli-kit/node/api/admin'
76
import {beforeEach, describe, expect, test, vi} from 'vitest'
87

9-
vi.mock('../auth/session-store.js')
108
vi.mock('../auth/session-lifecycle.js', () => ({loadStoredStoreSession: vi.fn()}))
11-
vi.mock('@shopify/cli-kit/node/api/admin', async () => {
12-
const actual = await vi.importActual<typeof import('@shopify/cli-kit/node/api/admin')>(
13-
'@shopify/cli-kit/node/api/admin',
14-
)
15-
return {
16-
...actual,
17-
fetchApiVersions: vi.fn(),
18-
}
19-
})
9+
vi.mock('./admin-transport.js', () => ({
10+
fetchPublicApiVersions: vi.fn(),
11+
// runAdminStoreGraphQLOperation isn't exercised here, but we re-export it for type completeness.
12+
runAdminStoreGraphQLOperation: vi.fn(),
13+
}))
2014

2115
describe('prepareAdminStoreGraphQLContext', () => {
2216
const store = 'shop.myshopify.com'
@@ -32,26 +26,23 @@ describe('prepareAdminStoreGraphQLContext', () => {
3226

3327
beforeEach(() => {
3428
vi.mocked(loadStoredStoreSession).mockResolvedValue(storedSession)
35-
vi.mocked(fetchApiVersions).mockResolvedValue([
29+
vi.mocked(fetchPublicApiVersions).mockResolvedValue([
3630
{handle: '2025-10', supported: true},
3731
{handle: '2025-07', supported: true},
3832
{handle: 'unstable', supported: false},
39-
] as any)
33+
])
4034
})
4135

4236
test('returns the stored admin session, version, and full auth session', async () => {
4337
const result = await prepareAdminStoreGraphQLContext({store})
4438

4539
expect(loadStoredStoreSession).toHaveBeenCalledWith(store)
46-
expect(fetchApiVersions).toHaveBeenCalledWith({
47-
token: 'token',
48-
storeFqdn: store,
40+
expect(fetchPublicApiVersions).toHaveBeenCalledWith({
41+
adminSession: {token: 'token', storeFqdn: store},
42+
session: storedSession,
4943
})
5044
expect(result).toEqual({
51-
adminSession: {
52-
token: 'token',
53-
storeFqdn: store,
54-
},
45+
adminSession: {token: 'token', storeFqdn: store},
5546
version: '2025-10',
5647
session: storedSession,
5748
})
@@ -68,9 +59,9 @@ describe('prepareAdminStoreGraphQLContext', () => {
6859

6960
const result = await prepareAdminStoreGraphQLContext({store})
7061

71-
expect(fetchApiVersions).toHaveBeenCalledWith({
72-
token: 'fresh-token',
73-
storeFqdn: store,
62+
expect(fetchPublicApiVersions).toHaveBeenCalledWith({
63+
adminSession: {token: 'fresh-token', storeFqdn: store},
64+
session: refreshedSession,
7465
})
7566
expect(result.adminSession.token).toBe('fresh-token')
7667
expect(result.session).toEqual(refreshedSession)
@@ -82,36 +73,27 @@ describe('prepareAdminStoreGraphQLContext', () => {
8273
expect(result.version).toBe('2025-07')
8374
})
8475

85-
test('allows unstable without validating against fetched versions', async () => {
76+
test('allows unstable without consulting the transport, but still loads the stored session', async () => {
8677
const result = await prepareAdminStoreGraphQLContext({store, userSpecifiedVersion: 'unstable'})
8778

88-
expect(result.version).toBe('unstable')
89-
expect(fetchApiVersions).not.toHaveBeenCalled()
90-
})
91-
92-
test('clears the current stored auth and prompts re-auth with real scopes when API version lookup gets invalid auth', async () => {
93-
vi.mocked(fetchApiVersions).mockRejectedValue(
94-
new AbortError(`Error connecting to your store ${store}: unauthorized 401 {}`),
95-
)
96-
97-
await expect(prepareAdminStoreGraphQLContext({store})).rejects.toMatchObject({
98-
message: `Stored app authentication for ${store} is no longer valid.`,
99-
tryMessage: 'To re-authenticate, run:',
100-
nextSteps: [[{command: `shopify store auth --store ${store} --scopes read_products,write_orders`}]],
79+
expect(loadStoredStoreSession).toHaveBeenCalledWith(store)
80+
expect(result).toEqual({
81+
adminSession: {token: 'token', storeFqdn: store},
82+
version: 'unstable',
83+
session: storedSession,
10184
})
102-
expect(clearStoredStoreAppSession).toHaveBeenCalledWith(store, '42')
103-
})
104-
105-
test('rethrows unrelated API version lookup failures', async () => {
106-
vi.mocked(fetchApiVersions).mockRejectedValue(new AbortError('upstream exploded'))
107-
108-
await expect(prepareAdminStoreGraphQLContext({store})).rejects.toThrow('upstream exploded')
109-
expect(clearStoredStoreAppSession).not.toHaveBeenCalled()
85+
expect(fetchPublicApiVersions).not.toHaveBeenCalled()
11086
})
11187

11288
test('throws when the requested API version is invalid', async () => {
11389
await expect(prepareAdminStoreGraphQLContext({store, userSpecifiedVersion: '1999-01'})).rejects.toThrow(
11490
'Invalid API version',
11591
)
11692
})
93+
94+
test('rethrows whatever the transport raises (errors are owned by the transport)', async () => {
95+
vi.mocked(fetchPublicApiVersions).mockRejectedValue(new AbortError('upstream exploded'))
96+
97+
await expect(prepareAdminStoreGraphQLContext({store})).rejects.toThrow('upstream exploded')
98+
})
11799
})

packages/store/src/cli/services/store/execute/admin-context.ts

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
1-
import {throwReauthenticateStoreAuthError} from '../auth/recovery.js'
2-
import {clearStoredStoreAppSession} from '../auth/session-store.js'
1+
import {fetchPublicApiVersions} from './admin-transport.js'
32
import {loadStoredStoreSession} from '../auth/session-lifecycle.js'
4-
import {fetchApiVersions} from '@shopify/cli-kit/node/api/admin'
53
import {AbortError} from '@shopify/cli-kit/node/error'
64
import type {AdminSession} from '@shopify/cli-kit/node/session'
75
import type {StoredStoreAppSession} from '../auth/session-store.js'
@@ -21,25 +19,7 @@ async function resolveApiVersion(options: {
2119

2220
if (userSpecifiedVersion === 'unstable') return userSpecifiedVersion
2321

24-
let availableVersions
25-
try {
26-
availableVersions = await fetchApiVersions(adminSession)
27-
} catch (error) {
28-
if (
29-
error instanceof AbortError &&
30-
error.message.includes(`Error connecting to your store ${adminSession.storeFqdn}:`) &&
31-
/\b(?:401|404)\b/.test(error.message)
32-
) {
33-
clearStoredStoreAppSession(session.store, session.userId)
34-
throwReauthenticateStoreAuthError(
35-
`Stored app authentication for ${session.store} is no longer valid.`,
36-
session.store,
37-
session.scopes.join(','),
38-
)
39-
}
40-
41-
throw error
42-
}
22+
const availableVersions = await fetchPublicApiVersions({adminSession, session})
4323

4424
if (!userSpecifiedVersion) {
4525
const supportedVersions = availableVersions.filter((version) => version.supported).map((version) => version.handle)

0 commit comments

Comments
 (0)