Skip to content

fix: Added exception re-raise in httpx method overwrites#867

Open
Rafal-Chrzanowski-IBM wants to merge 3 commits intoinstana:mainfrom
Rafal-Chrzanowski-IBM:main
Open

fix: Added exception re-raise in httpx method overwrites#867
Rafal-Chrzanowski-IBM wants to merge 3 commits intoinstana:mainfrom
Rafal-Chrzanowski-IBM:main

Conversation

@Rafal-Chrzanowski-IBM
Copy link
Copy Markdown

Fixes #866.

Added exception re-raise after exception has been recorded.
Previously pylance showed the following warning:

Function with declared return type "Response" must return value on all code paths
  "None" is not assignable to "Response"

Currently this warning isn't visible anymore.

@Rafal-Chrzanowski-IBM Rafal-Chrzanowski-IBM requested a review from a team as a code owner April 22, 2026 08:22
@github-actions
Copy link
Copy Markdown

@Rafal-Chrzanowski-IBM the signed-off-by was not found in the following 1 commits:

  • a6dd88d: fix: Added exception re-raise in httpx method overwrites

📝 What should I do to fix it?

All proposed commits should include a sign-off in their messages, ideally at the end.

❔ Why it is required

The Developer Certificate of Origin (DCO) is a lightweight way for contributors to certify that they wrote or otherwise have the right to submit the code they are contributing to the project. Here is the full text of the DCO, reformatted for readability:

By making a contribution to this project, I certify that:

a. The contribution was created in whole or in part by me and I have the right to submit it under the open source license indicated in the file; or

b. The contribution is based upon previous work that, to the best of my knowledge, is covered under an appropriate open source license and I have the right under that license to submit that work with modifications, whether created in whole or in part by me, under the same open source license (unless I am permitted to submit under a different license), as indicated in the file; or

c. The contribution was provided directly to me by some other person who certified (a), (b) or (c) and I have not modified it.

d. I understand and agree that this project and the contribution are public and that a record of the contribution (including all personal information I submit with it, including my sign-off) is maintained indefinitely and may be redistributed consistent with this project or the open source license(s) involved.

Contributors sign-off that they adhere to these requirements by adding a Signed-off-by line to commit messages.

This is my commit message

Signed-off-by: Random Developer <randomdeveloper@example.com>

Git even has a -s command line option to append this automatically to your commit message:

$ git commit -s -m 'This is my commit message'

Signed-off-by: Rafal Chrzanowski <Rafal.Chrzanowski@ibm.com>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi @Rafal-Chrzanowski-IBM, thanks for the PR!
According to our spec, we aren't supposed to raise exceptions that are caused by our instrumentation code. But I understand that we shouldn't suppress the exceptions coming from the underlying client library as well. Could you please re-raise the exception only for the original wrapped call as in the snippet I've shared below?

Please update the code (for both the methods) and verify from your end if it fixes the issue. We can then review the PR again, thanks!

            try:
                request = args[0]
                _set_request_span_attributes(span, request)  # Has its own try-except
                tracer.inject(span.context, Format.HTTP_HEADERS, request.headers)
            except Exception:
                logger.debug("httpx handle_request_with_instana:", exc_info=True)
            
            try:
                response = wrapped(*args, **kwargs)
            except Exception as e:
                span.record_exception(e)
                raise
            
            _set_response_span_attributes(span, response)  # Has its own try-except

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Hi @GSVarsha, thank you for taking a look so quickly. I updated the code according to your suggestions. Please let me know whether I should make any further changes to the PR. Thank you.

Signed-off-by: Rafal Chrzanowski <Rafal.Chrzanowski@ibm.com>
Signed-off-by: Rafal Chrzanowski <Rafal.Chrzanowski@ibm.com>
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.

[Bug]: httpx handle_request_with_instana returns None instead of re-raising exception

2 participants