diff --git a/src/errors.ts b/src/errors.ts index 0e2f32d..b67cc54 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -21,7 +21,10 @@ export class HaloPsaError extends Error { } /** - * Authentication error (400 bad credentials, 401 unauthorized) + * Authentication error (401 unauthorized; also 400 from the OAuth token + * endpoint itself, thrown directly by AuthManager — never by HttpClient, + * since resource requests never carry skipAuth and so never reach + * HttpClient's own 400 branch, see HaloPsaBadRequestError) */ export class HaloPsaAuthenticationError extends HaloPsaError { constructor(message: string, statusCode: number = 401, response?: unknown) { @@ -68,6 +71,22 @@ export class HaloPsaValidationError extends HaloPsaError { } } +/** + * Bad request error (400 from a resource endpoint that isn't a recognized + * validation-error shape — a malformed or incomplete request payload, not + * a credentials problem. Resource requests are always authenticated via a + * Bearer token by the time they reach here, so a plain 400 here can't be + * "bad credentials" the way it legitimately can be on the OAuth token + * endpoint itself; see HaloPsaAuthenticationError) + */ +export class HaloPsaBadRequestError extends HaloPsaError { + constructor(message: string, response?: unknown) { + super(message, 400, response); + this.name = 'HaloPsaBadRequestError'; + Object.setPrototypeOf(this, HaloPsaBadRequestError.prototype); + } +} + /** * Rate limit exceeded error (429) */ diff --git a/src/http.ts b/src/http.ts index b078481..13a0b97 100644 --- a/src/http.ts +++ b/src/http.ts @@ -8,6 +8,7 @@ import type { RateLimiter } from './rate-limiter.js'; import { HaloPsaError, HaloPsaAuthenticationError, + HaloPsaBadRequestError, HaloPsaForbiddenError, HaloPsaNotFoundError, HaloPsaValidationError, @@ -163,14 +164,20 @@ export class HttpClient { switch (response.status) { case 400: - // Could be bad credentials on token request or validation error + // Every request that reaches here is a resource call (skipAuth is + // never true outside this file — the OAuth token endpoint is + // fetched directly by AuthManager, not through HttpClient), so a + // 400 here is never a credentials problem: the Bearer token, if + // wrong, fails as a 401 below, not a 400. This is the request body + // itself being rejected — a missing/invalid field HaloPSA's + // server-side validation requires but this SDK doesn't mark + // `required` (e.g. Actions' `outcome`), or similar. if (this.isValidationError(responseBody)) { const errors = this.parseValidationErrors(responseBody); throw new HaloPsaValidationError('Validation error', errors, responseBody); } - throw new HaloPsaAuthenticationError( - 'Bad request - invalid credentials or parameters', - 400, + throw new HaloPsaBadRequestError( + `Bad request (400): ${method} ${url} rejected the request parameters`, responseBody ); diff --git a/src/index.ts b/src/index.ts index 30f1a6f..324aa7c 100644 --- a/src/index.ts +++ b/src/index.ts @@ -14,6 +14,7 @@ export { DEFAULT_RATE_LIMIT_CONFIG } from './config.js'; export { HaloPsaError, HaloPsaAuthenticationError, + HaloPsaBadRequestError, HaloPsaForbiddenError, HaloPsaNotFoundError, HaloPsaValidationError, diff --git a/tests/unit/http.test.ts b/tests/unit/http.test.ts index 856d5e1..fd9dfa7 100644 --- a/tests/unit/http.test.ts +++ b/tests/unit/http.test.ts @@ -14,8 +14,11 @@ import { AuthManager } from '../../src/auth.js'; import { RateLimiter } from '../../src/rate-limiter.js'; import { HaloPsaError, + HaloPsaAuthenticationError, + HaloPsaBadRequestError, HaloPsaNotFoundError, HaloPsaServerError, + HaloPsaValidationError, } from '../../src/errors.js'; import type { ResolvedConfig } from '../../src/config.js'; @@ -124,6 +127,37 @@ describe('HttpClient response handling', () => { expect((err as HaloPsaServerError).response).toEqual({ message: 'boom' }); }, 15000); + it('a non-validation-shaped 400 raises HaloPsaBadRequestError, not an auth error', async () => { + // Regression: node-halopsa#78 — a 400 from a resource endpoint (e.g. a + // required field like Actions' `outcome` missing) used to throw + // HaloPsaAuthenticationError with a "invalid credentials or parameters" + // message, which read as a permissions/credentials problem to callers + // even though the Bearer token was never in question — a bad token + // fails as 401, not 400. This body has neither `errors` nor + // `validation_errors`, so it isn't the recognized validation shape. + vi.mocked(fetch).mockResolvedValue( + realResponse('{"message":"outcome is required"}', { status: 400 }) + ); + const err = await makeClient() + .request('/Actions', { method: 'POST', body: [{ ticket_id: 1, note: 'hi' }] }) + .catch((e: unknown) => e); + expect(err).toBeInstanceOf(HaloPsaBadRequestError); + expect(err).not.toBeInstanceOf(HaloPsaAuthenticationError); + expect((err as HaloPsaBadRequestError).message).not.toMatch(/credentials/i); + expect((err as HaloPsaBadRequestError).response).toEqual({ message: 'outcome is required' }); + }); + + it('a validation-shaped 400 still raises HaloPsaValidationError', async () => { + vi.mocked(fetch).mockResolvedValue( + realResponse('{"errors":[{"field":"outcome","message":"is required"}]}', { status: 400 }) + ); + const err = await makeClient() + .request('/Actions', { method: 'POST', body: [{ ticket_id: 1, note: 'hi' }] }) + .catch((e: unknown) => e); + expect(err).toBeInstanceOf(HaloPsaValidationError); + expect((err as HaloPsaValidationError).errors).toEqual([{ field: 'outcome', message: 'is required' }]); + }); + it('generic non-2xx statuses raise HaloPsaError with the raw body', async () => { vi.mocked(fetch).mockResolvedValue( realResponse('teapot', { status: 418, headers: { 'content-type': 'text/plain' } })