Skip to content

onStatusCodes is respected during fallbacks #1239

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jul 25, 2025
Merged

Conversation

roh26it
Copy link
Collaborator

@roh26it roh26it commented Jul 23, 2025

Description

onStatusCodes is now respected during fallbacks

Copy link

matter-code-review bot commented Jul 23, 2025

Code Quality bug fix reliability maintainability

Description

Summary By MatterAI MatterAI logo

🔄 What Changed

This pull request refines the fallback logic within the tryTargetsRecursively function in src/handlers/handlerUtils.ts. The conditional logic for breaking out of the fallback loop now explicitly incorporates checks for currentTarget.strategy?.onStatusCodes and the x-portkey-gateway-exception header. Previously, only a generic response?.headers.get('x-portkey-gateway-exception') === 'true' was checked, and onStatusCodes was not fully integrated into the fallback decision. The updated logic now correctly respects the onStatusCodes array, ensuring that if a response status is not included in the specified codes, the fallback mechanism is skipped. Additionally, it explicitly checks for the x-portkey-gateway-exception header, preventing further retries when a gateway-level error has occurred.

🔍 Impact of the Change

This modification ensures that the gateway's fallback mechanism behaves more intelligently and predictably. It prevents unnecessary retries or fallbacks when a specific status code indicates a successful or handled response, or when a gateway-level exception signals a non-recoverable error. This leads to more efficient resource utilization and clearer error propagation, improving the reliability and precision of multi-target request handling.

📁 Total Files Changed

  • src/handlers/handlerUtils.ts (Modified: 10 additions, 5 deletions)

🧪 Test Added

N/A

🔒Security Vulnerabilities

None detected. The changes are focused on refining existing fallback logic and do not introduce new attack vectors or expose sensitive information.

Motivation

This change is motivated by the need to ensure that the onStatusCodes configuration is fully respected during fallback attempts, and to explicitly handle gateway exceptions to prevent redundant retries, thereby improving the robustness and configurability of the request routing logic.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Refactoring (no functional changes)

How Has This Been Tested?

  • Unit Tests
  • Integration Tests
  • Manual Testing

Screenshots (if applicable)

N/A

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Related Issues

.

Tip

Quality Recommendations

  1. Refactor the complex conditional logic within the if statement for improved readability and maintainability. Consider extracting parts of the condition into well-named boolean variables or helper functions to enhance clarity.

  2. Add logging statements to indicate when a fallback is skipped due to onStatusCodes not matching or an x-portkey-gateway-exception being present. This would enhance observability and aid in debugging unexpected behavior in production.

Copy link

This PR correctly fixes an issue with the onStatusCodes check in the fallback mechanism. The addition of the length check is a good defensive programming practice to ensure the array exists before trying to use its methods.

Copy link

Important

PR Review Skipped

PR review skipped as per the configuration setting. Run a manually review by commenting /matter review

💡Tips to use Matter AI

Command List

  • /matter summary: Generate AI Summary for the PR
  • /matter review: Generate AI Reviews for the latest commit in the PR
  • /matter review-full: Generate AI Reviews for the complete PR
  • /matter release-notes: Generate AI release-notes for the PR
  • /matter : Chat with your PR with Matter AI Agent
  • /matter remember : Generate AI memories for the PR
  • /matter explain: Get an explanation of the PR
  • /matter help: Show the list of available commands and documentation
  • Need help? Join our Discord server: https://discord.gg/fJU5DvanU3

@VisargD VisargD merged commit ae7a615 into main Jul 25, 2025
1 check passed
@VisargD VisargD deleted the fix/fallback_on_status_codes branch July 25, 2025 10:43
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.

2 participants