Skip to content

Conversation

@narengogi
Copy link
Collaborator

The return value from this is not used anyways, and it's throwing 500 error in cases like request body is used inside the getEndpoint() function etc

(the return value is not used anyways)
@narengogi narengogi requested a review from VisargD May 2, 2025 11:32
@matter-code-review
Copy link
Contributor

Code Quality bug fix

Summary By MatterAI MatterAI logo

🔄 What Changed

This PR modifies the tryPost function in handlerUtils.ts to avoid calling getEndpoint() when the function type is 'proxy'. Instead, it sets the endpoint to an empty string for proxy routes since the return value isn't used anyway.

🔍 Impact of the Change

Prevents 500 errors that were occurring when the request body was used inside the getEndpoint() function for proxy routes. The change improves stability by avoiding unnecessary function calls.

📁 Total Files Changed

1 file changed with 11 additions and 8 deletions (19 changes total).

🧪 Test Added

N/A - No tests were added in this PR.

🔒Security Vulnerabilities

N/A - No security vulnerabilities were addressed in this PR.

Motivation

The PR fixes 500 errors that occur when the request body is used inside the getEndpoint() function for proxy routes. Since the return value from getEndpoint() is not used for proxy routes, this change prevents unnecessary function calls that could cause errors.

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

N/A

Sequence Diagram

sequenceDiagram
    participant Client
    participant tryPost in handlerUtils.ts
    participant apiConfig
    
    Client->>tryPost in handlerUtils.ts: Request with fn parameter
    Note over tryPost in handlerUtils.ts: Check if fn is 'proxy'
    alt fn === 'proxy'
        tryPost in handlerUtils.ts->>tryPost in handlerUtils.ts: Set endpoint = ''
    else fn !== 'proxy'
        tryPost in handlerUtils.ts->>apiConfig: getEndpoint({
            c,
            providerOptions,
            fn,
            gatewayRequestBodyJSON,
            gatewayRequestBody: {},
            gatewayRequestURL
        })
        apiConfig-->>tryPost in handlerUtils.ts: Return endpoint
    end
    Note over tryPost in handlerUtils.ts: Continue processing with endpoint value
Loading

@matter-code-review
Copy link
Contributor

Code Quality bug fix

Summary By MatterAI MatterAI logo

🔄 What Changed

Modified the tryPost function in src/handlers/handlerUtils.ts to avoid calling getEndpoint() when the function type is 'proxy'. For non-proxy routes, the function now passes an empty object for gatewayRequestBody since the return value isn't used anywhere.

🔍 Impact of the Change

Prevents 500 errors that were occurring when the request body was being used inside the getEndpoint() function for proxy routes. Since the return value from getEndpoint() isn't used for proxy routes, this change optimizes the code by avoiding an unnecessary function call.

📁 Total Files Changed

1 file changed with 11 additions and 8 deletions (19 changes total).

🧪 Test Added

N/A - No tests were added in this PR.

🔒Security Vulnerabilities

N/A - No security vulnerabilities were addressed in this PR.

Motivation

Fix 500 errors that occur when the request body is used inside the getEndpoint() function for proxy routes. The return value from getEndpoint() is not used for proxy routes anyway.

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

Fixes 500 errors when request body is used inside the getEndpoint() function for proxy routes.

Sequence Diagram

sequenceDiagram
    participant Client
    participant tryPost as tryPost function
    participant apiConfig as apiConfig.getEndpoint
    
    Client->>tryPost: Request with fn parameter
    Note over tryPost: Check if fn === 'proxy'
    alt fn === 'proxy'
        tryPost->>tryPost: Set endpoint = ''
        Note over tryPost: Skip calling getEndpoint()
    else fn !== 'proxy'
        tryPost->>apiConfig: getEndpoint({
            c,
            providerOptions,
            fn,
            gatewayRequestBodyJSON,
            gatewayRequestBody: {},
            gatewayRequestURL
        })
        apiConfig-->>tryPost: Return endpoint
    end
    Note over tryPost: Continue with request processing
Loading

@VisargD VisargD merged commit cb94415 into Portkey-AI:main May 5, 2025
1 check 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.

2 participants