diff --git a/packages/core/src/apierror/apierror.ts b/packages/core/src/apierror/apierror.ts index 24363380..eff5ca52 100644 --- a/packages/core/src/apierror/apierror.ts +++ b/packages/core/src/apierror/apierror.ts @@ -1,6 +1,6 @@ import {z} from 'zod'; -import {Code, codeFromString} from './codes'; +import {Code} from './codes'; import type {ErrorDetails} from './details'; import {parseErrorDetails} from './details'; @@ -43,7 +43,9 @@ interface ApiErrorOptions { /** ApiError is a transport-agnostic error representing a Databricks API error. */ export class ApiError extends Error { - /** The canonical error code of the error. */ + /** + * The error code of the error. + */ readonly code: Code; /** @@ -130,7 +132,7 @@ export class ApiError extends Error { if (body === undefined || body.length === 0) { return new ApiError({ - code: toCode(statusCode), + code: Code.UNKNOWN, message: '', details: emptyDetails, httpStatusCode: statusCode, @@ -148,7 +150,7 @@ export class ApiError extends Error { // error does not come directly from a Databricks API. A typical example // is when the error is returned by a proxy. return new ApiError({ - code: toCode(statusCode), + code: Code.UNKNOWN, message: '', details: emptyDetails, httpStatusCode: statusCode, @@ -161,7 +163,7 @@ export class ApiError extends Error { const result = errorResponseSchema.safeParse(parsed); if (!result.success) { return new ApiError({ - code: toCode(statusCode), + code: Code.UNKNOWN, message: '', details: emptyDetails, httpStatusCode: statusCode, @@ -173,15 +175,17 @@ export class ApiError extends Error { const errResp = result.data; - // Error codes may be missing or be an integer (legacy APIs). In such - // cases, defer to the HTTP status code to infer the closest canonical - // error code. - let errorCode: Code; - if (typeof errResp.error_code === 'string') { - errorCode = codeFromString(errResp.error_code); - } else { - errorCode = toCode(statusCode); - } + // code carries the error_code string verbatim: a canonical code (e.g. + // "NOT_FOUND") matches a named Code member, while a Databricks + // product-specific code (e.g. "CATALOG_DOES_NOT_EXIST") is an open Code + // value. It is Code.UNKNOWN when the response carries no string error_code + // (missing or an integer); the HTTP status is never used to infer a code, + // since it may not reflect the true error semantic, so callers fall back to + // httpStatusCode. + const code: Code = + typeof errResp.error_code === 'string' && errResp.error_code !== '' + ? errResp.error_code + : Code.UNKNOWN; // Determine the error message from available fields. let errorMessage = ''; @@ -196,7 +200,7 @@ export class ApiError extends Error { } return new ApiError({ - code: errorCode, + code, message: errorMessage, details: parseErrorDetails(errResp.details), httpStatusCode: statusCode, @@ -205,49 +209,3 @@ export class ApiError extends Error { }); } } - -// Maps an HTTP status code to the closest canonical error code. -export function toCode(httpCode: number): Code { - // Canonical mappings. - switch (httpCode) { - case 200: - return Code.OK; - case 400: - return Code.INVALID_ARGUMENT; - case 401: - return Code.UNAUTHENTICATED; - case 403: - return Code.PERMISSION_DENIED; - case 404: - return Code.NOT_FOUND; - case 409: - return Code.ABORTED; - case 416: - return Code.OUT_OF_RANGE; - case 429: - return Code.RESOURCE_EXHAUSTED; - case 501: - return Code.UNIMPLEMENTED; - case 503: - return Code.UNAVAILABLE; - case 504: - return Code.DEADLINE_EXCEEDED; - default: - break; - } - - // Fallback for status codes without a direct canonical mapping. - if (httpCode >= 200 && httpCode < 300) { - return Code.OK; - } - if (httpCode >= 400 && httpCode < 500) { - // Most non-canonical 4xx status codes are state related and map - // to the definition of FailedPrecondition. - return Code.FAILED_PRECONDITION; - } - if (httpCode >= 500 && httpCode < 600) { - return Code.INTERNAL; - } - - return Code.UNKNOWN; -} diff --git a/packages/core/src/apierror/codes/codes.ts b/packages/core/src/apierror/codes/codes.ts index 8b27907a..6ee06d5a 100644 --- a/packages/core/src/apierror/codes/codes.ts +++ b/packages/core/src/apierror/codes/codes.ts @@ -1,28 +1,20 @@ /** - * Defines error codes for API errors and their retry semantics. - * - * @packageDocumentation + * Code is the error code carried by an API error. */ - -/** - * Code is a numeric code for an error. - * - * The numeric values are stable and can be relied upon across SDK versions. - */ -enum Code { +// eslint-disable-next-line @typescript-eslint/naming-convention -- Enum-style const object. +export const Code = { /** - * Unknown indicates an error that cannot be classified. - * - * This code might be used for malformed error responses or error responses - * using an error code that cannot be mapped to a code in this package. + * Unknown indicates an error that cannot be classified. It is used for + * malformed error responses and for responses that carry no string error + * code (missing or an integer). */ - UNKNOWN = 0, + UNKNOWN: 'UNKNOWN', /** OK indicates the operation completed successfully. */ - OK = 1, + OK: 'OK', - /** Canceled indicates the operation was canceled (typically by the caller). */ - CANCELED = 2, + /** Cancelled indicates the operation was cancelled (typically by the caller). */ + CANCELLED: 'CANCELLED', /** * InvalidArgument indicates the client specified an invalid argument. @@ -31,7 +23,7 @@ enum Code { * that are problematic regardless of the state of the system. For example, * a malformed request parameter. */ - INVALID_ARGUMENT = 3, + INVALID_ARGUMENT: 'INVALID_ARGUMENT', /** * DeadlineExceeded means the operation expired before completion. @@ -41,19 +33,19 @@ enum Code { * example, a successful response from a server could have been delayed * long enough for the deadline to expire. */ - DEADLINE_EXCEEDED = 4, + DEADLINE_EXCEEDED: 'DEADLINE_EXCEEDED', /** * NotFound means a requested entity (e.g. a resource or a file) was * not found. */ - NOT_FOUND = 5, + NOT_FOUND: 'NOT_FOUND', /** * AlreadyExists means an attempt to create an entity failed because one * already exists. */ - ALREADY_EXISTS = 6, + ALREADY_EXISTS: 'ALREADY_EXISTS', /** * PermissionDenied indicates the caller does not have permission to @@ -63,13 +55,13 @@ enum Code { * some resource (e.g. too many requests) which is a ResourceExhausted * error. */ - PERMISSION_DENIED = 7, + PERMISSION_DENIED: 'PERMISSION_DENIED', /** * ResourceExhausted indicates some resource has been exhausted, perhaps * a per-user quota, or perhaps the entire file system is out of space. */ - RESOURCE_EXHAUSTED = 8, + RESOURCE_EXHAUSTED: 'RESOURCE_EXHAUSTED', /** * FailedPrecondition indicates the operation was rejected because the @@ -77,14 +69,14 @@ enum Code { * For example, directory to be deleted may be non-empty, an rmdir * operation is applied to a non-directory, etc. */ - FAILED_PRECONDITION = 9, + FAILED_PRECONDITION: 'FAILED_PRECONDITION', /** * Aborted indicates the operation was aborted, typically due to a * concurrency issue like sequencer check failures, transaction aborts, * etc. */ - ABORTED = 10, + ABORTED: 'ABORTED', /** * OutOfRange means the operation was attempted past the valid range. @@ -103,20 +95,20 @@ enum Code { * a space can easily look for an OutOfRange error to detect when * they are done. */ - OUT_OF_RANGE = 11, + OUT_OF_RANGE: 'OUT_OF_RANGE', /** * Unimplemented indicates the operation is not implemented or not * supported/enabled in this service. */ - UNIMPLEMENTED = 12, + UNIMPLEMENTED: 'UNIMPLEMENTED', /** * Internal indicates an internal error. This means some invariants * expected by the underlying system have been broken. If you see * this error, something is very broken. */ - INTERNAL = 13, + INTERNAL: 'INTERNAL', /** * Unavailable indicates the service is currently unavailable. @@ -128,62 +120,16 @@ enum Code { * The Databricks SDK will generally automatically retry the request * with a backoff when encountering this error. */ - UNAVAILABLE = 14, + UNAVAILABLE: 'UNAVAILABLE', /** DataLoss indicates unrecoverable data loss or corruption. */ - DATA_LOSS = 15, + DATA_LOSS: 'DATA_LOSS', /** * Unauthenticated indicates the request does not have valid * authentication credentials for the operation. */ - UNAUTHENTICATED = 16, -} - -// Maps Code values to their canonical string representation. -const CODE_TO_STRING: ReadonlyMap = new Map([ - [Code.UNKNOWN, 'UNKNOWN'], - [Code.OK, 'OK'], - [Code.CANCELED, 'CANCELLED'], - [Code.INVALID_ARGUMENT, 'INVALID_ARGUMENT'], - [Code.DEADLINE_EXCEEDED, 'DEADLINE_EXCEEDED'], - [Code.NOT_FOUND, 'NOT_FOUND'], - [Code.ALREADY_EXISTS, 'ALREADY_EXISTS'], - [Code.PERMISSION_DENIED, 'PERMISSION_DENIED'], - [Code.RESOURCE_EXHAUSTED, 'RESOURCE_EXHAUSTED'], - [Code.FAILED_PRECONDITION, 'FAILED_PRECONDITION'], - [Code.ABORTED, 'ABORTED'], - [Code.OUT_OF_RANGE, 'OUT_OF_RANGE'], - [Code.UNIMPLEMENTED, 'UNIMPLEMENTED'], - [Code.INTERNAL, 'INTERNAL'], - [Code.UNAVAILABLE, 'UNAVAILABLE'], - [Code.DATA_LOSS, 'DATA_LOSS'], - [Code.UNAUTHENTICATED, 'UNAUTHENTICATED'], -]); - -// Maps canonical strings back to Code values. -const STRING_TO_CODE: ReadonlyMap = new Map( - [...CODE_TO_STRING.entries()].map(([code, str]) => [str, code]) -); - -/** - * Returns the canonical string representation of an error code. - * - * If the code is not recognized, "UNKNOWN" is returned. Note that - * Code.CANCELED maps to "CANCELLED" (British spelling) to match the gRPC - * convention. - */ -function codeToString(code: Code): string { - return CODE_TO_STRING.get(code) ?? 'UNKNOWN'; -} - -/** - * Converts a string representation of an error code to its corresponding - * Code value. If the string does not match any known code, Code.UNKNOWN is - * returned. - */ -function codeFromString(s: string): Code { - return STRING_TO_CODE.get(s) ?? Code.UNKNOWN; -} + UNAUTHENTICATED: 'UNAUTHENTICATED', +} as const; -export {Code, codeToString, codeFromString}; +export type Code = (typeof Code)[keyof typeof Code] | (string & {}); diff --git a/packages/core/src/apierror/codes/index.ts b/packages/core/src/apierror/codes/index.ts index 9bc58432..2d5b2e63 100644 --- a/packages/core/src/apierror/codes/index.ts +++ b/packages/core/src/apierror/codes/index.ts @@ -4,4 +4,4 @@ * @packageDocumentation */ -export {Code, codeToString, codeFromString} from './codes'; +export {Code} from './codes'; diff --git a/packages/core/tests/apierror/apierror.test.ts b/packages/core/tests/apierror/apierror.test.ts index d635254b..4802fcdc 100644 --- a/packages/core/tests/apierror/apierror.test.ts +++ b/packages/core/tests/apierror/apierror.test.ts @@ -1,5 +1,5 @@ import {describe, it, expect} from 'vitest'; -import {ApiError, toCode} from '../../src/apierror/apierror'; +import {ApiError} from '../../src/apierror/apierror'; import {Code} from '../../src/apierror/codes'; import type {ErrorDetails} from '../../src/apierror/details'; @@ -144,7 +144,7 @@ describe('fromHttpError', () => { desc: 'empty body with status', statusCode: 400, want: new ApiError({ - code: Code.INVALID_ARGUMENT, + code: Code.UNKNOWN, message: '', details: emptyDetails, }), @@ -154,7 +154,7 @@ describe('fromHttpError', () => { statusCode: 404, header: new Headers({'Content-Type': 'application/json'}), want: new ApiError({ - code: Code.NOT_FOUND, + code: Code.UNKNOWN, message: '', details: emptyDetails, }), @@ -164,7 +164,7 @@ describe('fromHttpError', () => { statusCode: 502, body: encode('Bad Gateway'), want: new ApiError({ - code: Code.INTERNAL, + code: Code.UNKNOWN, message: '', details: emptyDetails, }), @@ -174,7 +174,7 @@ describe('fromHttpError', () => { statusCode: 400, body: encode('{not valid json'), want: new ApiError({ - code: Code.INVALID_ARGUMENT, + code: Code.UNKNOWN, message: '', details: emptyDetails, }), @@ -221,14 +221,14 @@ describe('fromHttpError', () => { }), }, { - desc: 'standard error with unknown error_code', - statusCode: 400, + desc: 'Databricks-specific error_code is carried verbatim as the code', + statusCode: 404, body: encode( - '{"error_code": "SOME_UNKNOWN_CODE", "message": "Something went wrong"}' + '{"error_code": "CATALOG_DOES_NOT_EXIST", "message": "Catalog not found"}' ), want: new ApiError({ - code: Code.UNKNOWN, - message: 'Something went wrong', + code: 'CATALOG_DOES_NOT_EXIST', + message: 'Catalog not found', details: emptyDetails, }), }, @@ -237,7 +237,7 @@ describe('fromHttpError', () => { statusCode: 403, body: encode('{"message": "Access denied"}'), want: new ApiError({ - code: Code.PERMISSION_DENIED, + code: Code.UNKNOWN, message: 'Access denied', details: emptyDetails, }), @@ -247,7 +247,7 @@ describe('fromHttpError', () => { statusCode: 400, body: encode('{"error_code": 42, "message": "Invalid request"}'), want: new ApiError({ - code: Code.INVALID_ARGUMENT, + code: Code.UNKNOWN, message: 'Invalid request', details: emptyDetails, }), @@ -257,7 +257,7 @@ describe('fromHttpError', () => { statusCode: 400, body: encode('{"error": "Invalid parameter"}'), want: new ApiError({ - code: Code.INVALID_ARGUMENT, + code: Code.UNKNOWN, message: 'Invalid parameter', details: emptyDetails, }), @@ -267,7 +267,7 @@ describe('fromHttpError', () => { statusCode: 400, body: encode('{"message": "New message", "error": "Old error"}'), want: new ApiError({ - code: Code.INVALID_ARGUMENT, + code: Code.UNKNOWN, message: 'New message', details: emptyDetails, }), @@ -277,7 +277,7 @@ describe('fromHttpError', () => { statusCode: 404, body: encode('{"detail": "User not found", "scimType": "invalidValue"}'), want: new ApiError({ - code: Code.NOT_FOUND, + code: Code.UNKNOWN, message: 'User not found', details: emptyDetails, }), @@ -287,7 +287,7 @@ describe('fromHttpError', () => { statusCode: 400, body: encode('{"scimType": "uniqueness"}'), want: new ApiError({ - code: Code.INVALID_ARGUMENT, + code: Code.UNKNOWN, message: 'uniqueness', details: emptyDetails, }), @@ -297,7 +297,7 @@ describe('fromHttpError', () => { statusCode: 400, body: encode('{"message": "Standard message", "detail": "SCIM detail"}'), want: new ApiError({ - code: Code.INVALID_ARGUMENT, + code: Code.UNKNOWN, message: 'Standard message', details: emptyDetails, }), @@ -328,45 +328,3 @@ describe('fromHttpError', () => { } }); }); - -describe('toCode', () => { - const testCases: { - httpCode: number; - want: Code; - }[] = [ - // Direct mappings. - {httpCode: 200, want: Code.OK}, - {httpCode: 400, want: Code.INVALID_ARGUMENT}, - {httpCode: 401, want: Code.UNAUTHENTICATED}, - {httpCode: 403, want: Code.PERMISSION_DENIED}, - {httpCode: 404, want: Code.NOT_FOUND}, - {httpCode: 409, want: Code.ABORTED}, - {httpCode: 416, want: Code.OUT_OF_RANGE}, - {httpCode: 429, want: Code.RESOURCE_EXHAUSTED}, - {httpCode: 504, want: Code.DEADLINE_EXCEEDED}, - {httpCode: 501, want: Code.UNIMPLEMENTED}, - {httpCode: 503, want: Code.UNAVAILABLE}, - - // Fallback ranges. - {httpCode: 201, want: Code.OK}, - {httpCode: 204, want: Code.OK}, - {httpCode: 418, want: Code.FAILED_PRECONDITION}, - {httpCode: 500, want: Code.INTERNAL}, - {httpCode: 599, want: Code.INTERNAL}, - - // Unknown (valid). - {httpCode: 100, want: Code.UNKNOWN}, - {httpCode: 300, want: Code.UNKNOWN}, - - // Unknown (invalid). - {httpCode: -1, want: Code.UNKNOWN}, - {httpCode: 0, want: Code.UNKNOWN}, - {httpCode: 42, want: Code.UNKNOWN}, - {httpCode: 600, want: Code.UNKNOWN}, - {httpCode: 1337, want: Code.UNKNOWN}, - ]; - - it.each(testCases)('status $httpCode', ({httpCode, want}) => { - expect(toCode(httpCode)).toBe(want); - }); -}); diff --git a/packages/core/tests/apierror/codes/codes.test.ts b/packages/core/tests/apierror/codes/codes.test.ts index e07482e9..83c1330f 100644 --- a/packages/core/tests/apierror/codes/codes.test.ts +++ b/packages/core/tests/apierror/codes/codes.test.ts @@ -1,37 +1,19 @@ import {describe, it, expect} from 'vitest'; -import { - Code, - codeToString, - codeFromString, -} from '../../../src/apierror/codes/codes'; - -const ALL_CODES = Object.values(Code).filter( - (v): v is Code => typeof v === 'number' -); +import {Code} from '../../../src/apierror/codes/codes'; describe('Code', () => { - describe('round-trip', () => { - it.each(ALL_CODES)( - 'should round-trip code %i through string conversion', - code => { - expect(codeFromString(codeToString(code))).toBe(code); - } - ); - }); - - describe('codeToString', () => { - it('should return "UNKNOWN" for unrecognized code values', () => { - // Numeric enums accept arbitrary numbers at runtime. - expect(codeToString(999 as Code)).toBe('UNKNOWN'); - expect(codeToString(-1 as Code)).toBe('UNKNOWN'); - }); + it('exposes the canonical gRPC codes as their string names', () => { + expect(Code.UNKNOWN).toBe('UNKNOWN'); + expect(Code.OK).toBe('OK'); + expect(Code.CANCELLED).toBe('CANCELLED'); + expect(Code.NOT_FOUND).toBe('NOT_FOUND'); + expect(Code.INVALID_ARGUMENT).toBe('INVALID_ARGUMENT'); + expect(Code.UNAUTHENTICATED).toBe('UNAUTHENTICATED'); }); - describe('codeFromString', () => { - it('should return Code.UNKNOWN for unrecognized strings', () => { - expect(codeFromString('NONEXISTENT')).toBe(Code.UNKNOWN); - expect(codeFromString('')).toBe(Code.UNKNOWN); - expect(codeFromString('not_found')).toBe(Code.UNKNOWN); - }); + it('is an open enum that also accepts product-specific codes', () => { + // A Databricks-specific code that is not a named member is still a Code. + const code: Code = 'CATALOG_DOES_NOT_EXIST'; + expect(code).toBe('CATALOG_DOES_NOT_EXIST'); }); }); diff --git a/packages/examples/src/demos/auth-and-errors.ts b/packages/examples/src/demos/auth-and-errors.ts index fab50507..a4ea1e65 100644 --- a/packages/examples/src/demos/auth-and-errors.ts +++ b/packages/examples/src/demos/auth-and-errors.ts @@ -14,7 +14,6 @@ import {newPatCredentials} from '@databricks/sdk-auth/credentials'; import {ApiError} from '@databricks/sdk-core/apierror'; -import {codeToString} from '@databricks/sdk-core/apierror/codes'; import {LogLevel} from '@databricks/sdk-core/logger'; const log = new LogLevel('debug'); @@ -86,7 +85,7 @@ async function runErrorHandling(): Promise { ); const err404 = await toApiError(resp404); if (err404) { - log.info(' Code: ', codeToString(err404.code)); + log.info(' Code: ', err404.code); log.info(' HTTP Status:', err404.httpStatusCode); log.info(' Message: ', err404.message); if (err404.details.errorInfo) { @@ -108,7 +107,7 @@ async function runErrorHandling(): Promise { }); const err401 = await toApiError(resp401); if (err401) { - log.info(' Code: ', codeToString(err401.code)); + log.info(' Code: ', err401.code); log.info(' HTTP Status:', err401.httpStatusCode); log.info(' Message: ', err401.message); }