Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Customising raise_for_status #3864

Closed
samuelcolvin opened this issue Jun 24, 2019 · 4 comments
Closed

Customising raise_for_status #3864

samuelcolvin opened this issue Jun 24, 2019 · 4 comments

Comments

@samuelcolvin
Copy link
Member

samuelcolvin commented Jun 24, 2019

It would be great if it was possible to customize raise_for_status to:

Adding all this to aiohttp would obviously be infeasible, but it would be possible to add another kwarg when creating a client session response_check - a coroutine called when raise_for_status=True to check if a response is valid.

This approach would be backwards compatible.

It would also be useful if check_response were also called for request timeouts, but maybe that's more complicated and would not be backwards compatible.

Information which might be useful when debugging http client requests:

  • request url
  • redirects
  • request headers
  • request body
  • response headers
  • response body
  • response time

I don't want to add all these to the standard error, but allowing a coroutine to check responses would allow all this to added if I wanted.

@asvetlov
Copy link
Member

The feature request sounds reasonable.

Should response_check parameter to be added for every request() / get() / post() method or to ClientSession constructor only?

The real question is: want we add yet another callback into our API?
Inversion of Control is hard to debug.
Are there alternative approaches?

Don't get me wrong: I'm not strongly against the proposal but want to discuss it carefully first.

@samuelcolvin
Copy link
Member Author

I understand.

Technically this is not another callback, just overwriting _raise_for_status with a custom method I think.

@samuelcolvin
Copy link
Member Author

response_check should definitely be added to ClientSession, if I want custom logic on a request I can do it after response as usual.

samuelcolvin added a commit that referenced this issue Sep 9, 2019
* implement response check, #3864

* switch to raise_for_status coroutine

* fix linting

* revert name corrections

* fix types, add request tests

* change and docs

* allow non callable truey raise_for_status
@asvetlov
Copy link
Member

Fixed by #3892

frederikaalund pushed a commit to sbtinstruments/aiohttp that referenced this issue Nov 26, 2019
* implement response check, aio-libs#3864

* switch to raise_for_status coroutine

* fix linting

* revert name corrections

* fix types, add request tests

* change and docs

* allow non callable truey raise_for_status
frederikaalund pushed a commit to sbtinstruments/aiohttp that referenced this issue Feb 7, 2021
* implement response check, aio-libs#3864

* switch to raise_for_status coroutine

* fix linting

* revert name corrections

* fix types, add request tests

* change and docs

* allow non callable truey raise_for_status
arjd pushed a commit to arjd/aiohttp that referenced this issue May 2, 2022
* implement response check, aio-libs#3864

* switch to raise_for_status coroutine

* fix linting

* revert name corrections

* fix types, add request tests

* change and docs

* allow non callable truey raise_for_status

(cherry picked from commit e5beaca)
Dreamsorcerer pushed a commit that referenced this issue May 3, 2022
…892 implement response check (#6731)

* implement response check (#3892)

* implement response check, #3864

* switch to raise_for_status coroutine

* fix linting

* revert name corrections

* fix types, add request tests

* change and docs

* allow non callable truey raise_for_status

(cherry picked from commit e5beaca)

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* remove noqa

Co-authored-by: Samuel Colvin <s@muelcolvin.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants