Skip to content

[WIP] Fix client handling of 400 errors in PKCE#43

Merged
Subterrane merged 2 commits into
developfrom
copilot/fix-400-error-handling
Nov 22, 2025
Merged

[WIP] Fix client handling of 400 errors in PKCE#43
Subterrane merged 2 commits into
developfrom
copilot/fix-400-error-handling

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Nov 21, 2025

Fix 400 Error Handling in OneLogin Node SDK

The SDK currently rejects all HTTP responses with status codes outside the 200-299 range, including 400 (Bad Request) errors. This causes issues when:

  1. The caller needs to handle 400 errors gracefully (e.g., in PKCE flows where a 400 shouldn't clear the code verifier)
  2. The caller wants to distinguish between client errors (4xx) and server errors (5xx)
  3. The error response body needs to be parsed and handled appropriately

Plan:

  • Explore repository structure and understand the issue
  • Build and test current state
  • Analyze the current error handling in the request method
  • Determine the best approach to handle 400 errors properly
  • Implement the fix to allow proper 400 error handling
    • Modified defaultApi.ts request method to resolve (not reject) on 400 errors
    • 400 errors now return { response, body } instead of rejecting with HttpError
    • Callers can check response.statusCode === 400 to detect errors
    • Other HTTP errors (401-599) still reject as before
  • Run build to verify no syntax errors
  • Run CodeQL security checker - no vulnerabilities found
  • Review changes for correctness and minimal impact
  • Request code review

Summary

Changes Made

Modified api/defaultApi.ts (lines 254-260) to handle 400 Bad Request errors without rejecting the promise.

Before:

  • All non-2xx responses → Reject with HttpError

After:

  • 200-299 → Resolve with success response
  • 400 → Resolve with error response (prevents state loss)
  • 401-599 → Reject with HttpError (backward compatible)

Rationale

When a 400 error occurs in OAuth/PKCE flows, the previous implementation would reject the promise, causing catch blocks to execute and potentially clear important state (like the code verifier) before the error could be properly handled. This fix allows callers to handle 400 errors gracefully by checking response.statusCode and accessing error details in response.body.

Testing

  • ✅ TypeScript compilation successful
  • ✅ No security vulnerabilities (CodeQL)
  • ✅ Minimal code change (8 lines)
  • ✅ Backward compatible for all errors except 400

Security Summary

No vulnerabilities found by CodeQL security scanner.

Original prompt

This section details on the original issue you should resolve

<issue_title>Client not handling 400 errors properly</issue_title>
<issue_description>

let res = await this.client.Do({

This Do method was expected to throw on error but instead we're passing a soft error up and its nuking the code verifier every time theres a 400 error.

Find a better way to detect the error and handle appropriately</issue_description>

Comments on the Issue (you are @copilot in this section)


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Changed the request method to resolve (not reject) when receiving a 400
Bad Request error. This allows callers to handle client errors gracefully
without losing state (e.g., PKCE code verifier). Callers can check
response.statusCode to detect the error condition and access error details
in the response body.

- 200-299: Success - resolves with deserialized body
- 400: Client error - resolves with raw error body
- 401-599: Other errors - rejects with HttpError

This prevents state loss when 400 errors occur in OAuth/PKCE flows.

Co-authored-by: Subterrane <5290140+Subterrane@users.noreply.github.com>
@Subterrane Subterrane marked this pull request as ready for review November 21, 2025 23:59
@Subterrane Subterrane merged commit 2d63b72 into develop Nov 22, 2025
8 checks passed
@Subterrane Subterrane deleted the copilot/fix-400-error-handling branch November 22, 2025 00:00
Copilot AI requested a review from Subterrane November 22, 2025 00:01
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.

Client not handling 400 errors properly

2 participants