Skip to content

fix(sdk-python): avoid empty JSON body on GET#1606

Open
mahendrarathore1742 wants to merge 2 commits intolangfuse:mainfrom
mahendrarathore1742:fix/sdk-get-empty-body
Open

fix(sdk-python): avoid empty JSON body on GET#1606
mahendrarathore1742 wants to merge 2 commits intolangfuse:mainfrom
mahendrarathore1742:fix/sdk-get-empty-body

Conversation

@mahendrarathore1742
Copy link
Copy Markdown

@mahendrarathore1742 mahendrarathore1742 commented Apr 1, 2026

This PR fixes an issue where the SDK would send an empty JSON body ({}) with GET requests, which violates HTTP specs and is rejected by some load balancers (e.g., Google Cloud). Now, if no additional body parameters are provided, the request body is set to None for GET requests.

Fix: Avoid sending {} as the body for GET requests.
Adds unit tests to ensure correct request body behavior.

Disclaimer: Experimental PR review

Greptile Summary

This PR fixes a bug where GET requests could send an empty JSON body ({}) that is rejected by some load balancers (e.g., Google Cloud). The fix refactors maybe_filter_request_body to return None directly when no additional_body_parameters are present, rather than returning {} and relying on a downstream json_body != {} guard in get_request_body to convert it to None.

Key changes:

  • maybe_filter_request_body now short-circuits to None when request_options is None or when additional_body_parameters is absent/empty, making the intent explicit.
  • The downstream != {} safety check in get_request_body (lines 204–206) is retained and still useful for the non-None data mapping path, which is unchanged.
  • Three unit tests are added covering the main scenarios; a test for request_options=None would complete coverage of the new early-return path.

Confidence Score: 5/5

  • Safe to merge; the fix is correct and functionally equivalent while being cleaner, with only a minor test-coverage gap.
  • All remaining feedback is P2 (missing a single unit-test case for request_options=None). The core logic change is sound: maybe_filter_request_body previously returned {} for the no-body case which was already neutralised by the != {} check downstream, and the new code simply makes that None return explicit. No regressions are introduced.
  • No files require special attention beyond the minor test-coverage suggestion in tests/test_http_client.py.

Important Files Changed

Filename Overview
langfuse/api/core/http_client.py Refactored maybe_filter_request_body to return None directly when additional_body_parameters is absent/empty, replacing the indirect {} → None conversion that relied on a downstream != {} check in get_request_body. Logic is correct and cleaner.
tests/test_http_client.py New test file with three cases covering the fix; missing a request_options=None test for completeness, but all existing assertions are correct.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[get_request_body called] --> B{data is not None?}
    B -- Yes --> C[maybe_filter_request_body\nwith data]
    B -- No --> D[maybe_filter_request_body\nwith json]

    D --> E{request_options\nis None?}
    E -- Yes --> F[return None\nNEW short-circuit]
    E -- No --> G{additional_body_parameters\nfalsy or missing?}
    G -- Yes --> H[return None\nNEW short-circuit]
    G -- No --> I[return jsonable_encoder\nof params]

    C --> J[data_content merged dict]

    I --> K[json_body set]
    F --> K
    H --> K
    J --> L[data_body set]

    K --> M{json_body is empty dict?}
    M -- Yes --> N[return None\nsafety net]
    M -- No --> O[return json_body]
    L --> P{data_body is empty dict?}
    P -- Yes --> Q[return None\nsafety net]
    P -- No --> R[return data_body]

    style F fill:#90EE90
    style H fill:#90EE90
Loading

Reviews (1): Last reviewed commit: "fix(sdk-python): avoid empty JSON body o..." | Re-trigger Greptile

(2/5) Greptile learns from your feedback when you react with thumbs up/down!

Return None when no additional body parameters are provided so GET requests don't include {} payloads. Adds unit tests for request body handling.
Copy link
Copy Markdown

@claude claude bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 1, 2026

CLA assistant check
All committers have signed the CLA.

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