Skip to content

fix: do not panic on constructor param errors - #285

Merged
barjin merged 5 commits into
masterfrom
fix/better-error-handling-constructors
Oct 15, 2025
Merged

barjin merged 5 commits into
masterfrom
fix/better-error-handling-constructors

Conversation

@barjin

@barjin barjin commented Oct 15, 2025

Copy link
Copy Markdown
Member

Introduces better error handling for constructor parameter errors.

Closes #280

@barjin
barjin requested a review from Copilot October 15, 2025 13:22
@barjin barjin self-assigned this Oct 15, 2025
@github-actions github-actions Bot added this to the 125th sprint - Tooling team milestone Oct 15, 2025
@github-actions github-actions Bot added the t-tooling Issues with this label are in the ownership of the tooling team. label Oct 15, 2025

Copilot AI left a comment

Copy link
Copy Markdown

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 improves error handling in constructor parameters by replacing panics with proper Result returns. The core change propagates errors from the build() method instead of using unwrap(), preventing runtime panics when client construction fails.

Key Changes:

  • Modified ImpitBuilder::build() to return Result<Impit, ImpitError> instead of Impit
  • Enhanced error messages to include contextual information (URLs, proxy configurations)
  • Updated all language bindings (Python, Node.js) to handle the new error-returning API

Reviewed Changes

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

Show a summary per file
File Description
impit/src/impit.rs Changed build() return type to Result, propagated errors from new_reqwest_client(), added proxy error handling
impit/src/errors.rs Enhanced UrlParsingError to include the URL string, added From<reqwest::Error> implementation
impit-python/src/errors.rs Updated pattern matching for new UrlParsingError(String) variant
impit-python/src/client.rs Added error handling for build() result in synchronous client
impit-python/src/async_client.rs Added error handling for build() result in async client
impit-node/src/lib.rs Added error handling and mapping for build() result in Node.js binding

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread impit/src/impit.rs
Comment thread impit/src/impit.rs Outdated
Comment thread impit/src/impit.rs Outdated
Comment thread impit/src/impit.rs
Comment thread impit-python/src/client.rs Outdated
@barjin
barjin merged commit bb1210e into master Oct 15, 2025
59 of 60 checks passed
@barjin
barjin deleted the fix/better-error-handling-constructors branch October 15, 2025 16:30
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.

Unknown error when using impit-node; the process crashes immediately after an error is thrown.

2 participants