Skip to content

Commit

Permalink
Refactor: extract error utilities for consistency
Browse files Browse the repository at this point in the history
  • Loading branch information
frandiox committed Feb 7, 2025
1 parent ddfd841 commit 946e9e4
Show file tree
Hide file tree
Showing 7 changed files with 245 additions and 78 deletions.
150 changes: 150 additions & 0 deletions packages/theme/src/cli/utilities/errors.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
import {
renderThrownError,
createSyncingCatchError,
createAbortCatchError,
createFetchError,
extractFetchErrorInfo,
} from './errors.js'
import {describe, test, expect, vi} from 'vitest'
import {AbortError} from '@shopify/cli-kit/node/error'
import {renderError, renderFatalError} from '@shopify/cli-kit/node/ui'

vi.mock('@shopify/cli-kit/node/output')
vi.mock('@shopify/cli-kit/node/ui')

describe('errors', () => {
const mockExit = vi.spyOn(process, 'exit').mockImplementation(() => undefined as never)

describe('renderThrownError', () => {
test('renders AbortError with fatal error UI', () => {
const error = new AbortError('test error')
renderThrownError('Test Headline', error)

expect(renderFatalError).toHaveBeenCalledWith(expect.objectContaining({message: `Test Headline\n`}))
})

test('renders regular Error with error UI and debug output', () => {
const error = new Error('test error')
error.stack = 'test stack'
renderThrownError('Test Headline', error)

expect(renderError).toHaveBeenCalledWith({
headline: 'Test Headline',
body: 'test error',
})
})
})

describe('createSyncingCatchError', () => {
test('creates error handler for delete preset', () => {
const handler = createSyncingCatchError('test.liquid', 'delete')
const error = new Error('test error')

handler(error)

expect(renderError).toHaveBeenCalledWith({
headline: 'Failed to delete file "test.liquid" from remote theme.',
body: 'test error',
})
})

test('creates error handler for upload preset', () => {
const handler = createSyncingCatchError('test.liquid', 'upload')
const error = new Error('test error')

handler(error)

expect(renderError).toHaveBeenCalledWith({
headline: expect.stringMatching(/failed to upload/i),
body: 'test error',
})
})

test('creates error handler with custom headline', () => {
const handler = createSyncingCatchError('Custom Headline')
const error = new Error('test error')

handler(error)

expect(renderError).toHaveBeenCalledWith({
headline: 'Custom Headline',
body: 'test error',
})
})
})

describe('createAbortCatchError', () => {
test('creates error handler that exits process', () => {
const handler = createAbortCatchError('Test Headline')
const error = new Error('test error')

handler(error)

expect(renderError).toHaveBeenCalledWith({
headline: 'Test Headline',
body: 'test error',
})
expect(mockExit).toHaveBeenCalledWith(1)
})
})

describe('createFetchError', () => {
test('creates FetchError from Response-like object', () => {
const response = new Response('test error', {
status: 404,
statusText: 'Not Found',
headers: {'x-request-id': 'test-id'},
})

const responseUrl = 'https://test.com'
Object.defineProperty(response, 'url', {value: responseUrl})

expect(createFetchError(response)).toMatchObject({
statusCode: response.status,
statusMessage: response.statusText,
data: {
url: responseUrl,
requestId: response.headers.get('x-request-id'),
},
})
})

test('creates FetchError with defaults for missing properties', () => {
expect(createFetchError(new Error('test error'), 'https://error.com')).toMatchObject({
statusCode: 502,
statusMessage: 'Bad Gateway',
cause: expect.any(Error),
data: {url: 'https://error.com', requestId: undefined},
})
})
})

describe('extractFetchErrorInfo', () => {
test('extracts info from FetchError with all properties', () => {
const fetchError = createFetchError({
status: 404,
statusText: 'Not Found',
url: 'https://test.com',
headers: new Headers({'x-request-id': 'test-id'}),
})

expect(extractFetchErrorInfo(fetchError, 'Test context')).toMatchObject({
headline: expect.stringContaining('Test context'),
status: 404,
statusText: 'Not Found',
requestId: 'test-id',
url: 'https://test.com',
})
})

test('extracts info with defaults for regular Error', () => {
const error = new Error('test error')
const info = extractFetchErrorInfo(error)

expect(info.headline).toBe('Unexpected error during fetch with status 502 (Bad Gateway).')
expect(info.status).toBe(502)
expect(info.statusText).toBe('Bad Gateway')
expect(info.body).toBe(error.stack)
})
})
})
48 changes: 48 additions & 0 deletions packages/theme/src/cli/utilities/errors.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {AbortError} from '@shopify/cli-kit/node/error'
import {outputDebug} from '@shopify/cli-kit/node/output'
import {renderError, renderFatalError} from '@shopify/cli-kit/node/ui'
import {createError as createH3Error, type H3Error} from 'h3'

/**
* Renders an error banner for a thrown error with a headline.
Expand Down Expand Up @@ -47,3 +48,50 @@ export function createAbortCatchError(headline: string) {
process.exit(1)
}
}

/* ---- Fetch Errors ---- */
export type FetchError = H3Error<{requestId?: string; url?: string}>

/**
* Creates a FetchError from an actual Error object or a Response.
* @param resource - The error thrown by fetch, or the non-ok response.
* @param url - The URL of the request.
*/
export function createFetchError(resource: Partial<Response & Error>, url?: string | URL): FetchError {
return createH3Error({
status: resource.status ?? 502,
statusText: resource.statusText ?? 'Bad Gateway',
cause: resource,
data: {
url: (url ?? resource.url)?.toString(),
requestId: resource.headers?.get('x-request-id') ?? undefined,
},
})
}

/**
* Extracts information from a FetchError useful for rendering an error banner and creating responses.
* @param fetchError - The error to extract information from.
* @param context - The context description of the error.
*/
export function extractFetchErrorInfo(fetchError: Error | FetchError, context = 'Unexpected error during fetch') {
const error = fetchError as FetchError
const status = error.statusCode ?? 502
const statusText = error.statusMessage ?? 'Bad Gateway'

let headline = `${context.replace(/\.$/, '')} with status ${status} (${statusText}).`
if (error.data?.requestId) headline += `\nRequest ID: ${error.data.requestId}`
if (error.data?.url) headline += `\nURL: ${error.data.url}`

const cause = error.cause as undefined | Error

return {
headline,
body: cause?.stack ?? error.stack ?? error.message ?? statusText,
status,
statusText,
cause,
requestId: error.data?.requestId,
url: error.data?.url,
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import {getClientScripts, HotReloadEvent} from './client.js'
import {render} from '../storefront-renderer.js'
import {getExtensionInMemoryTemplates} from '../../theme-ext-environment/theme-ext-server.js'
import {patchRenderingResponse} from '../proxy.js'
import {createError, createEventStream, defineEventHandler, getProxyRequestHeaders, getQuery, type H3Error} from 'h3'
import {createFetchError, extractFetchErrorInfo} from '../../errors.js'
import {createEventStream, defineEventHandler, getProxyRequestHeaders, getQuery} from 'h3'
import {renderWarning} from '@shopify/cli-kit/node/ui'
import {extname, joinPath} from '@shopify/cli-kit/node/path'
import {parseJSON} from '@shopify/theme-check-node'
Expand Down Expand Up @@ -234,28 +235,17 @@ export function getHotReloadHandler(theme: Theme, ctx: DevServerContext) {
replaceExtensionTemplates: getExtensionInMemoryTemplates(ctx),
})
.then(async (response) => {
if (!response.ok) {
throw createError({
status: response.status,
statusText: response.statusText,
data: {requestId: response.headers.get('x-request-id'), url: response.url},
})
}
if (!response.ok) throw createFetchError(response)

return patchRenderingResponse(ctx, response)
})
.catch(async (error: H3Error<{requestId?: string; url?: string}>) => {
const status = error.statusCode ?? 502
const statusText = error.statusMessage ?? 'Bad Gateway'

if (!appBlockId) {
let headline = `Failed to render section on Hot Reload with status ${status} (${statusText}).`
if (error.data?.requestId) headline += `\nRequest ID: ${error.data.requestId}`
if (error.data?.url) headline += `\nURL: ${error.data.url}`
.catch(async (error: Error) => {
const {status, statusText, ...errorInfo} = extractFetchErrorInfo(
error,
'Failed to render section on Hot Reload',
)

const cause = error.cause as undefined | Error
renderWarning({headline, body: cause?.stack ?? error.stack ?? error.message})
}
if (!appBlockId) renderWarning(errorInfo)

return new Response(null, {status, statusText})
})
Expand Down
24 changes: 9 additions & 15 deletions packages/theme/src/cli/utilities/theme-environment/html.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@ import {getInMemoryTemplates, injectHotReloadScript} from './hot-reload/server.j
import {render} from './storefront-renderer.js'
import {getExtensionInMemoryTemplates} from '../theme-ext-environment/theme-ext-server.js'
import {logRequestLine} from '../log-request-line.js'
import {defineEventHandler, getCookie, type H3Error} from 'h3'
import {extractFetchErrorInfo} from '../errors.js'
import {defineEventHandler, getCookie} from 'h3'
import {renderError, renderFatalError} from '@shopify/cli-kit/node/ui'
import {AbortError} from '@shopify/cli-kit/node/error'
import {outputDebug} from '@shopify/cli-kit/node/output'
Expand Down Expand Up @@ -39,7 +40,7 @@ export function getHtmlHandler(theme: Theme, ctx: DevServerContext) {

// eslint-disable-next-line promise/no-nesting
const proxyResponse = await proxyStorefrontRequest(event, ctx).catch(
(error: H3Error) => new Response(null, {status: error.statusCode ?? 502}),
(error) => new Response(null, extractFetchErrorInfo(error)),
)

if (proxyResponse.status < 400) {
Expand All @@ -58,23 +59,16 @@ export function getHtmlHandler(theme: Theme, ctx: DevServerContext) {
return ctx.options.liveReload === 'off' ? body : injectHotReloadScript(body)
})
})
.catch(async (error: H3Error<{requestId?: string; url?: string}>) => {
const status = error.statusCode ?? 502
const statusText = error.statusMessage ?? 'Bad Gateway'
.catch(async (error) => {
const {status, statusText, cause, ...errorInfo} = extractFetchErrorInfo(error, 'Failed to render storefront')
renderError(errorInfo)

let headline = `Failed to render storefront with status ${status} (${statusText}).`
if (error.data?.requestId) headline += `\nRequest ID: ${error.data.requestId}`
if (error.data?.url) headline += `\nURL: ${error.data.url}`

const cause = error.cause as undefined | Error
renderError({headline, body: cause?.stack ?? error.stack ?? error.message})

const [title, ...rest] = headline.split('\n') as [string, ...string[]]
const [title, ...rest] = errorInfo.headline.split('\n') as [string, ...string[]]
let errorPageHtml = getErrorPage({
title,
header: title,
message: [...rest, cause?.message ?? error.message].join('<br>'),
code: error.stack?.replace(`${error.message}\n`, '') ?? '',
message: [...rest, cause?.message ?? ''].join('<br>'),
code: cause?.stack?.replace(`${error.message}\n`, '') ?? '',
})

if (ctx.options.liveReload !== 'off') {
Expand Down
48 changes: 17 additions & 31 deletions packages/theme/src/cli/utilities/theme-environment/proxy.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,9 @@
/* eslint-disable @typescript-eslint/no-non-null-assertion */
/* eslint-disable @typescript-eslint/no-dynamic-delete */
import {buildCookies} from './storefront-renderer.js'
import {logRequestLine} from '../log-request-line.js'
import {createFetchError, extractFetchErrorInfo} from '../errors.js'
import {renderWarning} from '@shopify/cli-kit/node/ui'
import {
defineEventHandler,
getRequestHeaders,
getRequestWebStream,
getRequestIP,
type H3Event,
createError,
H3Error,
} from 'h3'
import {defineEventHandler, getRequestHeaders, getRequestWebStream, getRequestIP, type H3Event} from 'h3'
import {extname} from '@shopify/cli-kit/node/path'
import {lookupMimeType} from '@shopify/cli-kit/node/mimes'
import type {Theme} from '@shopify/cli-kit/node/themes/types'
Expand Down Expand Up @@ -49,12 +41,14 @@ export function getProxyHandler(_theme: Theme, ctx: DevServerContext) {
}

if (canProxyRequest(event)) {
const pathname = event.path.split('?')[0] ?? ''

return proxyStorefrontRequest(event, ctx)
.then(async (response) => {
logRequestLine(event, response)

if (response.ok) {
const fileName = event.path.split('?')[0]?.split('/').at(-1) ?? ''
const fileName = pathname.split('/').at(-1) ?? ''
if (ctx.localThemeFileSystem.files.has(`assets/${fileName}.liquid`)) {
const newBody = injectCdnProxy(await response.text(), ctx)
return new Response(newBody, response)
Expand All @@ -63,20 +57,17 @@ export function getProxyHandler(_theme: Theme, ctx: DevServerContext) {

return response
})
.catch(async (error: H3Error) => {
const pathname = event.path.split('?')[0]!
if (error.statusCode >= 500 && !pathname.endsWith('.js.map')) {
const cause = error.cause as undefined | Error
renderWarning({
headline: `Failed to proxy request to ${pathname}`,
body: cause?.stack ?? error.stack ?? error.message,
})
.catch(async (error: Error) => {
const {status, statusText, ...errorInfo} = extractFetchErrorInfo(
error,
`Failed to proxy request to ${pathname}`,
)

if (status >= 500 && !pathname.endsWith('.js.map')) {
renderWarning(errorInfo)
}

return new Response(error.message, {
status: error.statusCode,
statusText: error.statusMessage,
})
return new Response(error.message, {status, statusText})
})
}
})
Expand Down Expand Up @@ -285,7 +276,7 @@ export function proxyStorefrontRequest(event: H3Event, ctx: DevServerContext): P
const headers = getProxyStorefrontHeaders(event)
const body = getRequestWebStream(event)

return fetch(url.toString(), {
return fetch(url, {
method: event.method,
body,
// @ts-expect-error Not included in official Node types
Expand All @@ -301,12 +292,7 @@ export function proxyStorefrontRequest(event: H3Event, ctx: DevServerContext): P
},
})
.then((response) => patchProxiedResponseHeaders(ctx, response))
.catch((error) => {
throw createError({
status: 502,
statusText: 'Bad Gateway',
data: {url},
cause: error,
})
.catch((error: Error) => {
throw createFetchError(error, url)
})
}
Loading

0 comments on commit 946e9e4

Please sign in to comment.