mirror of
https://github.com/Comfy-Org/ComfyUI_frontend.git
synced 2026-05-26 07:57:36 +00:00
341fef46a9
## Summary Replaces all 7 production `as Error` type assertions with proper `instanceof Error` narrowing or a new `toError()` helper, and adds an ESLint rule to prevent new ones. First slice of #11429 (the `as Error` category — 9 total occurrences, 7 production + 2 in a test file left untouched). ## Changes - **What**: - New `src/utils/errorUtil.ts` exporting `toError(value: unknown): Error` and `getErrorMessage(value: unknown): string | undefined`. `toError` returns the value unchanged if already an `Error`, otherwise wraps it (handles strings, `undefined`, JSON-serializable objects, and circular refs via `String()` fallback). - Refactored 7 production call sites: - `src/services/gateway/registrySearchGateway.ts` — `toError(error)` for `lastError` assignment in fallback loop - `src/platform/cloud/onboarding/auth.ts` (×2) — `toError(error)` for `captureApiError` Sentry calls - `src/renderer/extensions/vueNodes/widgets/composables/audio/useAudioRecorder.ts` — `toError(err)` before forwarding to `options.onError` - `src/extensions/core/load3d/LoaderManager.ts` — replaced `error as Error & { response?: ... }` cast inside `isNotFoundError` with `'response' in error` + nested narrowing - `apps/desktop-ui/src/stores/maintenanceTaskStore.ts` — inline `error instanceof Error ? error.message : String(error)` - `apps/desktop-ui/src/components/maintenance/TaskListPanel.vue` — inline `error instanceof Error ? error.message : undefined` - New ESLint rule (`no-restricted-syntax` block named `comfy/no-unsafe-error-assertion`) banning `TSAsExpression TSTypeReference[typeName.name='Error']` in `src/**` and `apps/*/src/**`, with test files (`*.test.ts`, `*.spec.ts`) excluded. - 12 unit tests for the new helpers in `src/utils/errorUtil.test.ts`. - **Breaking**: none - **Dependencies**: none ## Review Focus - The lint rule is scoped to non-test source files. Test files retain freedom to use `as Error` for fixture construction; only 2 occurrences exist (in `teamWorkspaceStore.test.ts` and `errorDialog.spec.ts`) and they're intentional. - `toError` is duplicated as inline `instanceof` narrowing in `apps/desktop-ui/` rather than imported, since the desktop-ui workspace doesn't share `@/utils/` with the main app and adding a path mapping for one helper felt heavier than two inline guards. - Remaining `as`-on-DOM categories (HTMLElement ×133, HTMLInputElement ×55, HTMLCanvasElement ×36, KeyboardEvent ×7, Element ×3, MouseEvent ×2, Event ×2) are intentionally left for follow-up PRs to keep this one reviewable. Refs #11429 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-11845-refactor-replace-unsafe-as-Error-assertions-with-type-guards-3546d73d36508137a015c4f9e8708f23) by [Unito](https://www.unito.io)
75 lines
2.3 KiB
TypeScript
75 lines
2.3 KiB
TypeScript
import { describe, expect, it } from 'vitest'
|
|
|
|
import { getErrorMessage, toError } from './errorUtil'
|
|
|
|
describe('toError', () => {
|
|
it('returns the same Error instance when given an Error', () => {
|
|
const err = new Error('boom')
|
|
expect(toError(err)).toBe(err)
|
|
})
|
|
|
|
it('preserves Error subclasses', () => {
|
|
class CustomError extends Error {}
|
|
const err = new CustomError('subclass')
|
|
expect(toError(err)).toBe(err)
|
|
expect(toError(err)).toBeInstanceOf(CustomError)
|
|
})
|
|
|
|
it('wraps a string as an Error message', () => {
|
|
const result = toError('plain string')
|
|
expect(result).toBeInstanceOf(Error)
|
|
expect(result.message).toBe('plain string')
|
|
})
|
|
|
|
it('wraps a number by stringifying it', () => {
|
|
const result = toError(42)
|
|
expect(result).toBeInstanceOf(Error)
|
|
expect(result.message).toBe('42')
|
|
})
|
|
|
|
it('wraps an object via JSON.stringify', () => {
|
|
const result = toError({ code: 'EBOOM', detail: 'nope' })
|
|
expect(result).toBeInstanceOf(Error)
|
|
expect(result.message).toBe('{"code":"EBOOM","detail":"nope"}')
|
|
})
|
|
|
|
it('falls back to String() when JSON.stringify throws (circular)', () => {
|
|
const obj: Record<string, unknown> = {}
|
|
obj.self = obj
|
|
const result = toError(obj)
|
|
expect(result).toBeInstanceOf(Error)
|
|
expect(result.message).toBe('[object Object]')
|
|
})
|
|
|
|
it('handles null and undefined', () => {
|
|
expect(toError(null).message).toBe('null')
|
|
expect(toError(undefined).message).toBe('undefined')
|
|
})
|
|
})
|
|
|
|
describe('getErrorMessage', () => {
|
|
it('returns the message of an Error', () => {
|
|
expect(getErrorMessage(new Error('boom'))).toBe('boom')
|
|
})
|
|
|
|
it('returns the value when given a string', () => {
|
|
expect(getErrorMessage('text')).toBe('text')
|
|
})
|
|
|
|
it('returns the message field of a plain object', () => {
|
|
expect(getErrorMessage({ message: 'from object' })).toBe('from object')
|
|
})
|
|
|
|
it('returns undefined for objects without a string message', () => {
|
|
expect(getErrorMessage({ code: 1 })).toBeUndefined()
|
|
expect(getErrorMessage({ message: 42 })).toBeUndefined()
|
|
})
|
|
|
|
it('returns undefined for null, undefined, numbers, booleans', () => {
|
|
expect(getErrorMessage(null)).toBeUndefined()
|
|
expect(getErrorMessage(undefined)).toBeUndefined()
|
|
expect(getErrorMessage(42)).toBeUndefined()
|
|
expect(getErrorMessage(true)).toBeUndefined()
|
|
})
|
|
})
|