Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion src/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
*/
Expand Down
15 changes: 11 additions & 4 deletions src/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import type { RateLimiter } from './rate-limiter.js';
import {
HaloPsaError,
HaloPsaAuthenticationError,
HaloPsaBadRequestError,
HaloPsaForbiddenError,
HaloPsaNotFoundError,
HaloPsaValidationError,
Expand Down Expand Up @@ -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
);

Expand Down
1 change: 1 addition & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export { DEFAULT_RATE_LIMIT_CONFIG } from './config.js';
export {
HaloPsaError,
HaloPsaAuthenticationError,
HaloPsaBadRequestError,
HaloPsaForbiddenError,
HaloPsaNotFoundError,
HaloPsaValidationError,
Expand Down
34 changes: 34 additions & 0 deletions tests/unit/http.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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' } })
Expand Down