Skip to content

fix: raise Python exception on response body read error#313

Merged
barjin merged 5 commits intomasterfrom
fix/raise-on-body-read-error
Nov 13, 2025
Merged

fix: raise Python exception on response body read error#313
barjin merged 5 commits intomasterfrom
fix/raise-on-body-read-error

Conversation

@barjin
Copy link
Copy Markdown
Member

@barjin barjin commented Nov 13, 2025

Originally, Python Impit bindings would return a response with an empty body on a body read error. This didn't make much sense and caused issues in the downstream dependencies. Now we rethrow the error so it can be properly handled.

Closes apify/apify-sdk-python#672

@barjin barjin requested a review from Copilot November 13, 2025 09:28
@barjin barjin self-assigned this Nov 13, 2025
@github-actions github-actions Bot added this to the 127th sprint - Tooling team milestone Nov 13, 2025
@github-actions github-actions Bot added the t-tooling Issues with this label are in the ownership of the tooling team. label Nov 13, 2025
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes the Python bindings to properly raise exceptions when response body read errors occur, instead of silently returning empty bodies. The key change is making the ImpitPyResponse::from_async method return a Result and propagating errors from the bytes() call.

Key Changes

  • Modified error handling infrastructure to support optional context information when errors occur without full request context
  • Changed ImpitPyResponse::from_async to return Result<Self, ImpitError> and propagate body read errors
  • Updated both sync and async client implementations to properly handle and convert the new error type

Reviewed Changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
impit/src/errors.rs Made ErrorContext fields optional and updated ImpitError::from to accept Option to support error creation without full context
impit/src/impit.rs Updated ErrorContext construction to wrap fields in Some() to match the new optional field structure
impit-python/src/response.rs Changed from_async to return Result and propagate errors from bytes() call instead of defaulting to empty body
impit-python/src/client.rs Updated error handling to properly convert ImpitError from from_async into ImpitPyError
impit-python/src/async_client.rs Updated error handling to properly convert ImpitError from from_async into PyErr
impit-python/Cargo.toml Added "auto-initialize" feature to pyo3 dependency
impit-python/uv.lock Bumped version from 0.8.0 to 0.9.0

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread impit/src/errors.rs
Comment thread impit/src/errors.rs
Comment thread impit/src/errors.rs
@barjin barjin merged commit 15a8ac7 into master Nov 13, 2025
53 checks passed
@barjin barjin deleted the fix/raise-on-body-read-error branch November 13, 2025 09:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

t-tooling Issues with this label are in the ownership of the tooling team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Investigate occasional JSONDecodeError in integration tests

3 participants