Skip to content

fix(http): stop labeling resource-endpoint 400s as credential errors - #79

Merged
asachs01 merged 1 commit into
mainfrom
fix/400-bad-request-not-auth-error
Sep 4, 2026
Merged

fix(http): stop labeling resource-endpoint 400s as credential errors#79
asachs01 merged 1 commit into
mainfrom
fix/400-bad-request-not-auth-error

Conversation

@asachs01

@asachs01 asachs01 commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Fixes 400 responses from non-token endpoints are mislabeled as credential errors #78 — a 400 from a resource endpoint (e.g. POST /Actions) was thrown as HaloPsaAuthenticationError with the message "Bad request - invalid credentials or parameters", even though skipAuth is never true outside HttpClient (the OAuth token endpoint is fetched directly by AuthManager, never through HttpClient), so every 400 reaching that branch is from an already-authenticated resource call — a bad token fails as 401, not 400.
  • Added HaloPsaBadRequestError for this case; the message now names the endpoint and says nothing about credentials. HaloPsaAuthenticationError keeps its 401 role plus the token endpoint's own legitimate 400 (unaffected — that's thrown separately in auth.ts).
  • We hit this in production via halopsa-mcp: a POST /Actions call missing outcome (not marked required in ActionCreateData, though HaloPSA's server apparently wants it) surfaced as an "invalid credentials" error and sent a customer down a permissions-troubleshooting path for a plain payload validation issue.

Test plan

  • New unit tests: a non-validation-shaped 400 → HaloPsaBadRequestError, not HaloPsaAuthenticationError, message has no "credentials" wording; a validation-shaped 400 still → HaloPsaValidationError (was previously untested)
  • Full suite: npm test — 271/271 passing
  • npm run lint — clean
  • npm run build — clean

🤖 Generated with Claude Code

https://claude.ai/code/session_01JgXg1NhHo6DvvAqoPowppi


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Summary by CodeRabbit

  • Bug Fixes

    • Corrected handling of HTTP 400 responses so resource-request errors are no longer incorrectly reported as authentication failures.
    • Validation-related 400 responses continue to be reported as validation errors.
  • New Features

    • Added the public HaloPsaBadRequestError for unrecognized or malformed resource-request responses, including relevant request details.

HttpClient.handleResponse() threw HaloPsaAuthenticationError with the
message "Bad request - invalid credentials or parameters" for any 400
response that wasn't in the recognized validation-error shape,
regardless of the actual endpoint. That's misleading: skipAuth is
never true outside this file (the OAuth token endpoint is fetched
directly by AuthManager, not through HttpClient), so every 400 that
reaches this branch is from an authenticated resource call, where a
bad/expired Bearer token already fails as 401, not 400. A 400 here is
always the request body itself being rejected.

We hit this via halopsa-mcp: a POST /Actions call missing a
server-required field (outcome isn't marked required in
ActionCreateData) came back as a plain 400, surfaced to the caller as
an "invalid credentials" error, and was read as an API-application
permissions problem — sending the customer down the wrong path
entirely.

Add HaloPsaBadRequestError for this case and use it instead. The
message now names the endpoint and says nothing about credentials.
HaloPsaAuthenticationError keeps its 401 role plus the token
endpoint's own legitimate 400 (thrown separately by AuthManager,
unaffected by this change).

Fixes #78

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JgXg1NhHo6DvvAqoPowppi
@coderabbitai

coderabbitai Bot commented Sep 4, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 206fcf21-219e-4357-b5c8-5d8842f935da

📥 Commits

Reviewing files that changed from the base of the PR and between e5b8e97 and 6bbbff2.

📒 Files selected for processing (4)
  • src/errors.ts
  • src/http.ts
  • src/index.ts
  • tests/unit/http.test.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

The HTTP client now classifies non-validation 400 responses from resource endpoints as HaloPsaBadRequestError. Authentication errors remain associated with 401 responses and OAuth token endpoint 400 responses. Validation-shaped 400 responses remain HaloPsaValidationError.

Changes

Resource 400 error handling

Layer / File(s) Summary
Bad request error contract and export
src/errors.ts, src/index.ts
Adds the public HaloPsaBadRequestError class and exports it. Authentication documentation now distinguishes OAuth token endpoint 400 responses from resource endpoint 400 responses.
HTTP 400 classification and regression coverage
src/http.ts, tests/unit/http.test.ts
Resource-level 400 responses without recognized validation fields now raise HaloPsaBadRequestError. Validation-shaped 400 responses continue to raise HaloPsaValidationError with parsed details.

Estimated code review effort: 2 (Simple) | ~15 minutes

Merge Risk: ⚪ Minimal · up to 6bbbf

Resource-endpoint 400 responses now use a neutral bad-request error while validation and authentication handling remain distinct. The updated behavior and regression coverage are ready to merge.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: resource-endpoint 400 responses no longer receive credential-error classification.
Linked Issues check ✅ Passed The changes satisfy issue #78. Non-validation-shaped 400 responses from resource endpoints now use HaloPsaBadRequestError, while validation-shaped responses remain HaloPsaValidationError. Token-endpoi…
Out of Scope Changes check ✅ Passed The added error class, public export, HTTP classification changes, and regression tests directly support the linked issue and stated objectives. No unrelated changes are identified.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 4…
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/400-bad-request-not-auth-error

Comment @coderabbitai help to get the list of available commands.

@asachs01
asachs01 merged commit 43a123f into main Sep 4, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

400 responses from non-token endpoints are mislabeled as credential errors

1 participant