Skip to content

Commit 1917b15

Browse files
MitchDickinsonMitchDickinson
committed
observability for failures to identity service
1 parent 656d299 commit 1917b15

File tree

2 files changed

+109
-15
lines changed

2 files changed

+109
-15
lines changed

‎packages/cli-kit/src/private/node/session/device-authorization.test.ts‎

Lines changed: 58 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -64,29 +64,81 @@ describe('requestDeviceAuthorization', () => {
6464
expect(got).toEqual(dataExpected)
6565
})
6666

67-
test('when the response is not valid JSON, throw an error', async () => {
67+
test('when the response is not valid JSON, throw an error with context', async () => {
6868
// Given
6969
const response = new Response('not valid JSON')
70+
Object.defineProperty(response, 'status', {value: 200})
71+
Object.defineProperty(response, 'statusText', {value: 'OK'})
7072
vi.mocked(shopifyFetch).mockResolvedValue(response)
7173
vi.mocked(identityFqdn).mockResolvedValue('fqdn.com')
7274
vi.mocked(clientId).mockReturnValue('clientId')
7375

7476
// When/Then
75-
await expect(requestDeviceAuthorization(['scope1', 'scope2'])).rejects.toThrow(
76-
'Received unexpected response from the authorization service. If this issue persists, please contact support at https://help.shopify.com',
77+
await expect(requestDeviceAuthorization(['scope1', 'scope2'])).rejects.toThrowError(
78+
'Received invalid response from authorization service (HTTP 200). Response could not be parsed as valid JSON. If this issue persists, please contact support at https://help.shopify.com',
7779
)
7880
})
7981

80-
test('when the response is empty, throw an error', async () => {
82+
test('when the response is empty, throw an error with empty body message', async () => {
8183
// Given
8284
const response = new Response('')
85+
Object.defineProperty(response, 'status', {value: 200})
86+
Object.defineProperty(response, 'statusText', {value: 'OK'})
8387
vi.mocked(shopifyFetch).mockResolvedValue(response)
8488
vi.mocked(identityFqdn).mockResolvedValue('fqdn.com')
8589
vi.mocked(clientId).mockReturnValue('clientId')
8690

8791
// When/Then
88-
await expect(requestDeviceAuthorization(['scope1', 'scope2'])).rejects.toThrow(
89-
'Received unexpected response from the authorization service. If this issue persists, please contact support at https://help.shopify.com',
92+
await expect(requestDeviceAuthorization(['scope1', 'scope2'])).rejects.toThrowError(
93+
'Received invalid response from authorization service (HTTP 200). Received empty response body. If this issue persists, please contact support at https://help.shopify.com',
94+
)
95+
})
96+
97+
test('when the response is HTML instead of JSON, throw an error with HTML detection', async () => {
98+
// Given
99+
const htmlResponse = '<!DOCTYPE html><html><body>Error page</body></html>'
100+
const response = new Response(htmlResponse)
101+
Object.defineProperty(response, 'status', {value: 404})
102+
Object.defineProperty(response, 'statusText', {value: 'Not Found'})
103+
vi.mocked(shopifyFetch).mockResolvedValue(response)
104+
vi.mocked(identityFqdn).mockResolvedValue('fqdn.com')
105+
vi.mocked(clientId).mockReturnValue('clientId')
106+
107+
// When/Then
108+
await expect(requestDeviceAuthorization(['scope1', 'scope2'])).rejects.toThrowError(
109+
'Received invalid response from authorization service (HTTP 404). The request may be malformed or unauthorized. Received HTML instead of JSON - the service endpoint may have changed. If this issue persists, please contact support at https://help.shopify.com',
110+
)
111+
})
112+
113+
test('when the server returns a 500 error with non-JSON response, throw an error with server issue message', async () => {
114+
// Given
115+
const response = new Response('Internal Server Error')
116+
Object.defineProperty(response, 'status', {value: 500})
117+
Object.defineProperty(response, 'statusText', {value: 'Internal Server Error'})
118+
vi.mocked(shopifyFetch).mockResolvedValue(response)
119+
vi.mocked(identityFqdn).mockResolvedValue('fqdn.com')
120+
vi.mocked(clientId).mockReturnValue('clientId')
121+
122+
// When/Then
123+
await expect(requestDeviceAuthorization(['scope1', 'scope2'])).rejects.toThrowError(
124+
'Received invalid response from authorization service (HTTP 500). The service may be experiencing issues. Response could not be parsed as valid JSON. If this issue persists, please contact support at https://help.shopify.com',
125+
)
126+
})
127+
128+
test('when response.text() fails, throw an error about network/streaming issue', async () => {
129+
// Given
130+
const response = new Response('some content')
131+
Object.defineProperty(response, 'status', {value: 200})
132+
Object.defineProperty(response, 'statusText', {value: 'OK'})
133+
// Mock text() to throw an error
134+
response.text = vi.fn().mockRejectedValue(new Error('Network error'))
135+
vi.mocked(shopifyFetch).mockResolvedValue(response)
136+
vi.mocked(identityFqdn).mockResolvedValue('fqdn.com')
137+
vi.mocked(clientId).mockReturnValue('clientId')
138+
139+
// When/Then
140+
await expect(requestDeviceAuthorization(['scope1', 'scope2'])).rejects.toThrowError(
141+
'Failed to read response from authorization service (HTTP 200). Network or streaming error occurred.',
90142
)
91143
})
92144
})

‎packages/cli-kit/src/private/node/session/device-authorization.ts‎

Lines changed: 51 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {AbortError, BugError} from '../../../public/node/error.js'
88
import {isCloudEnvironment} from '../../../public/node/context/local.js'
99
import {isCI, openURL} from '../../../public/node/system.js'
1010
import {isTTY, keypress} from '../../../public/node/ui.js'
11+
import {Response} from 'node-fetch'
1112

1213
export interface DeviceAuthorizationResponse {
1314
deviceCode: string
@@ -40,16 +41,28 @@ export async function requestDeviceAuthorization(scopes: string[]): Promise<Devi
4041
body: convertRequestToParams(queryParams),
4142
})
4243

43-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
44-
let jsonResult: any
44+
// First read the response body as text so we have it for debugging
45+
let responseText: string
4546
try {
46-
jsonResult = await response.json()
47+
responseText = await response.text()
4748
} catch (error) {
4849
throw new BugError(
49-
'Received unexpected response from the authorization service. If this issue persists, please contact support at https://help.shopify.com',
50+
`Failed to read response from authorization service (HTTP ${response.status}). Network or streaming error occurred.`,
51+
'Check your network connection and try again.',
5052
)
5153
}
5254

55+
// Now try to parse the text as JSON
56+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
57+
let jsonResult: any
58+
try {
59+
jsonResult = JSON.parse(responseText)
60+
} catch {
61+
// JSON.parse failed, handle the parsing error
62+
const errorMessage = buildAuthorizationParseErrorMessage(response, responseText)
63+
throw new BugError(errorMessage)
64+
}
65+
5366
outputDebug(outputContent`Received device authorization code: ${outputToken.json(jsonResult)}`)
5467
if (!jsonResult.device_code || !jsonResult.verification_uri_complete) {
5568
throw new BugError('Failed to start authorization process')
@@ -126,14 +139,12 @@ export async function pollForDeviceAuthorization(code: string, interval = 5): Pr
126139
}
127140
case 'slow_down':
128141
currentIntervalInSeconds += 5
129-
{
130-
startPolling()
131-
return
132-
}
142+
startPolling()
143+
return
133144
case 'access_denied':
134145
case 'expired_token':
135146
case 'unknown_failure': {
136-
reject(result)
147+
reject(new Error(`Device authorization failed: ${error}`))
137148
}
138149
}
139150
}
@@ -153,3 +164,34 @@ function convertRequestToParams(queryParams: {client_id: string; scope: string})
153164
.filter((hasValue) => Boolean(hasValue))
154165
.join('&')
155166
}
167+
168+
/**
169+
* Build a detailed error message for JSON parsing failures from the authorization service.
170+
* Provides context-specific error messages based on response status and content.
171+
*
172+
* @param response - The HTTP response object
173+
* @param responseText - The raw response body text
174+
* @returns Detailed error message about the failure
175+
*/
176+
function buildAuthorizationParseErrorMessage(response: Response, responseText: string): string {
177+
// Build helpful error message based on response status and content
178+
let errorMessage = `Received invalid response from authorization service (HTTP ${response.status}).`
179+
180+
// Add status-based context
181+
if (response.status >= 500) {
182+
errorMessage += ' The service may be experiencing issues.'
183+
} else if (response.status >= 400) {
184+
errorMessage += ' The request may be malformed or unauthorized.'
185+
}
186+
187+
// Add content-based context (check these regardless of status)
188+
if (responseText.trim().startsWith('<!DOCTYPE') || responseText.trim().startsWith('<html')) {
189+
errorMessage += ' Received HTML instead of JSON - the service endpoint may have changed.'
190+
} else if (responseText.trim() === '') {
191+
errorMessage += ' Received empty response body.'
192+
} else {
193+
errorMessage += ' Response could not be parsed as valid JSON.'
194+
}
195+
196+
return `${errorMessage} If this issue persists, please contact support at https://help.shopify.com`
197+
}

0 commit comments

Comments
 (0)