diff --git a/package-lock.json b/package-lock.json index 3a1c90b..2dc8e9a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -11,7 +11,7 @@ "license": "MIT", "dependencies": { "@doist/cli-core": "1.6.0", - "@doist/comms-sdk": "3.0.0", + "@doist/comms-sdk": "3.3.0", "@pnpm/tabtab": "0.5.4", "chalk": "5.6.2", "commander": "14.0.3", @@ -184,15 +184,15 @@ } }, "node_modules/@doist/comms-sdk": { - "version": "3.0.0", - "resolved": "https://registry.npmjs.org/@doist/comms-sdk/-/comms-sdk-3.0.0.tgz", - "integrity": "sha512-Rkwh/V8edTtY8AkiGNZt6GrS5mTiPOK3xYnX+MQT14g2nhsQPqjYhbba/QdF2QhwN4Pwm7+40wSbN93QKmXEVQ==", + "version": "3.3.0", + "resolved": "https://registry.npmjs.org/@doist/comms-sdk/-/comms-sdk-3.3.0.tgz", + "integrity": "sha512-RfD4xL8bhZLn9XAM646K2orYSV6CNGyZLN9ccB+A60eeY+FVJ2LiRebSxIcav7pRIyp625tqV5+GiN0cDBRtyg==", "license": "MIT", "dependencies": { "@doist/sdk-kmp": "0.2.3", "camelcase": "9.0.0", "ts-custom-error": "3.3.1", - "undici": "7.29.0", + "undici": "7.30.0", "uuid": "11.1.1", "zod": "4.4.3" }, @@ -9677,9 +9677,9 @@ } }, "node_modules/undici": { - "version": "7.29.0", - "resolved": "https://registry.npmjs.org/undici/-/undici-7.29.0.tgz", - "integrity": "sha512-IDxfleLmmbSskfWSUATiN1nfn2rDuvnMOqb5CWR92iIfojA0Ud+ulOAAEQ57LPr9rWmsreUyf5lwyao+7GNNVw==", + "version": "7.30.0", + "resolved": "https://registry.npmjs.org/undici/-/undici-7.30.0.tgz", + "integrity": "sha512-dkrQXeHSaoamnItlYbmzG0wFYrM0ZwDxCIg0A7aKjTyyhh9svRzCNFEzV+Vm05/yehjCzjDZ31KXfGEjYSztDQ==", "license": "MIT", "engines": { "node": ">=20.18.1" diff --git a/package.json b/package.json index 5af03a2..6a05c89 100644 --- a/package.json +++ b/package.json @@ -52,7 +52,7 @@ ], "dependencies": { "@doist/cli-core": "1.6.0", - "@doist/comms-sdk": "3.0.0", + "@doist/comms-sdk": "3.3.0", "@pnpm/tabtab": "0.5.4", "chalk": "5.6.2", "commander": "14.0.3", diff --git a/src/commands/migrate/migrate.test.ts b/src/commands/migrate/migrate.test.ts index 13a230d..94fbcfe 100644 --- a/src/commands/migrate/migrate.test.ts +++ b/src/commands/migrate/migrate.test.ts @@ -5,7 +5,9 @@ const sdkMocks = vi.hoisted(() => ({ fetchNewCommsUrls: vi.fn(), })) -vi.mock('@doist/comms-sdk', () => ({ +vi.mock('@doist/comms-sdk', async (importActual) => ({ + // errors.ts reads helpers from the SDK, so only the network call is faked. + ...(await importActual()), fetchNewCommsUrls: sdkMocks.fetchNewCommsUrls, })) diff --git a/src/lib/api.test.ts b/src/lib/api.test.ts index 53ebeb6..8546e5f 100644 --- a/src/lib/api.test.ts +++ b/src/lib/api.test.ts @@ -10,7 +10,10 @@ const sdkMocks = vi.hoisted(() => ({ addGroupUsers: vi.fn(), })) -vi.mock('@doist/comms-sdk', () => { +vi.mock('@doist/comms-sdk', async (importActual) => { + // Only the client is faked. The error class and the helpers that inspect it + // stay real, because the status mapping under test depends on them agreeing. + const actual = await importActual() class CommsApi { channels = { deleteChannel: sdkMocks.deleteChannel } attachments = { upload: sdkMocks.uploadAttachment } @@ -20,18 +23,7 @@ vi.mock('@doist/comms-sdk', () => { sdkMocks.createClient(token, options) } } - return { - CommsApi, - CommsRequestError: class CommsRequestError extends Error { - constructor( - message: string, - public httpStatusCode: number, - public responseData?: unknown, - ) { - super(message) - } - }, - } + return { ...actual, CommsApi } }) vi.mock('./auth.js', () => ({ diff --git a/src/lib/api.ts b/src/lib/api.ts index ff81f63..e9ca18a 100644 --- a/src/lib/api.ts +++ b/src/lib/api.ts @@ -1,22 +1,17 @@ import { CommsApi, + getCommsErrorString, type Group, + isConflict, + isMalformedId, + isNotFound, type User, type Workspace, type WorkspaceUser, } from '@doist/comms-sdk' import { getApiTokenSnapshot } from './auth.js' import { getConfig, updateConfig } from './config.js' -import { - CliError, - getCommsErrorString, - isConflict, - isForbidden, - isInsufficientScope, - isInvalidToken, - isMalformedId, - isNotFound, -} from './errors.js' +import { CliError, isForbidden, isInsufficientScope, isInvalidToken } from './errors.js' import { ensureMutationAllowed, isMutatingMethod } from './permissions.js' import { getProgressTracker } from './progress.js' import { withSpinner } from './spinner.js' diff --git a/src/lib/errors.test.ts b/src/lib/errors.test.ts index 8833d2f..3b00844 100644 --- a/src/lib/errors.test.ts +++ b/src/lib/errors.test.ts @@ -3,14 +3,10 @@ import { describe, expect, it } from 'vitest' import { CliError, - getCommsErrorString, isCliErrorCode, - isConflict, isForbidden, isInsufficientScope, isInvalidToken, - isMalformedId, - isNotFound, } from './errors.js' describe('isInsufficientScope', () => { @@ -129,46 +125,6 @@ describe('isInvalidToken', () => { }) }) -describe('isNotFound / isConflict', () => { - it('match on status alone', () => { - expect(isNotFound(new CommsRequestError('Request failed with status 404', 404, {}))).toBe( - true, - ) - expect(isConflict(new CommsRequestError('Request failed with status 409', 409, {}))).toBe( - true, - ) - expect(isNotFound(new CommsRequestError('Request failed with status 409', 409, {}))).toBe( - false, - ) - expect(isConflict(new CommsRequestError('Request failed with status 404', 404, {}))).toBe( - false, - ) - expect(isNotFound(new Error('something'))).toBe(false) - }) -}) - -describe('isMalformedId', () => { - it('is true only for the 409 the API sends for an id that does not decode', () => { - const malformed = new CommsRequestError('Request failed with status 409', 409, { - error_string: 'id must decode to 16 bytes. Regenerate the ID and retry.', - error_code: 217, - }) - expect(isMalformedId(malformed)).toBe(true) - expect(getCommsErrorString(malformed)).toBe( - 'id must decode to 16 bytes. Regenerate the ID and retry.', - ) - - const otherConflict = new CommsRequestError('Request failed with status 409', 409, { - error_string: 'Channel name already taken', - }) - expect(isMalformedId(otherConflict)).toBe(false) - expect( - isMalformedId(new CommsRequestError('Request failed with status 409', 409, {})), - ).toBe(false) - expect(getCommsErrorString(new CommsRequestError('x', 409, undefined))).toBeNull() - }) -}) - describe('isCliErrorCode', () => { it('matches a CliError by any of the given codes and nothing else', () => { const notFound = new CliError('NOT_FOUND', 'x') diff --git a/src/lib/errors.ts b/src/lib/errors.ts index 0123329..b42c108 100644 --- a/src/lib/errors.ts +++ b/src/lib/errors.ts @@ -1,4 +1,5 @@ import { CliError as BaseCliError, type CliErrorCode, type ErrorType } from '@doist/cli-core' +import { getCommsErrorString } from '@doist/comms-sdk' export { BaseCliError } export type { ErrorType } from '@doist/cli-core' @@ -83,7 +84,8 @@ function hasCommsStatusCode(error: unknown, status: number): error is { httpStat /** * Check whether an error is a Comms API 403 "Insufficient scope" response. - * Works with any error shaped like CommsRequestError (httpStatusCode + responseData). + * The status is read from any error carrying an `httpStatusCode`; the message + * only from a `CommsRequestError`. */ export function isInsufficientScope(error: unknown): boolean { return ( @@ -118,41 +120,6 @@ export function isCliErrorCode(error: unknown, ...codes: ErrorCode[]): boolean { return error instanceof CliError && codes.includes(error.code) } -export function isNotFound(error: unknown): boolean { - return hasCommsStatusCode(error, 404) -} - -export function isConflict(error: unknown): boolean { - return hasCommsStatusCode(error, 409) -} - -function getCommsResponseField(error: unknown, field: string): unknown { - if (typeof error !== 'object' || error === null || !('responseData' in error)) return undefined - const data = error.responseData - if (typeof data !== 'object' || data === null || !(field in data)) return undefined - return (data as Record)[field] -} - -/** The server's `error_string`, when the response body carried one. */ -export function getCommsErrorString(error: unknown): string | null { - const value = getCommsResponseField(error, 'error_string') - return typeof value === 'string' ? value : null -} - -/** The server's numeric `error_code`, when the response body carried one. */ -export function getCommsErrorCode(error: unknown): number | null { - const value = getCommsResponseField(error, 'error_code') - return typeof value === 'number' ? value : null -} - -/** - * Comms answers 409 with error_code 217 when an id does not base58-decode to - * 16 bytes. That is a bad reference, not a conflict. - */ -export function isMalformedId(error: unknown): boolean { - return isConflict(error) && getCommsErrorCode(error) === 217 -} - /** * Comms-flavoured CliError that preserves the historical positional * `(code, message, hints?, type?)` signature used across hundreds of call