aio-libs / aio-libs/aiohttp

Discuss HTTPException and children design

オープン
#3,757 コメント 3 件 リアクション 0 件 担当者 0 名 GitHub で見る
enhancement
主要言語
Python
スター
16.5k
フォーク
2.4k
平均マージ
17時間 22分
マージ済み PR(30日)
212

説明

Follow-up discussion for #3420 (https://github.com/aio-libs/aiohttp/pull/3420#issuecomment-491523997)

> Hm...that's quite a question.

> @asvetlov How we should behave in this case: we have a REST service (remember, REST is not about JSON) which accepts some binary data among the others (let's say it's the same application/protobuf) and have to respond with the similar one. Quite strange to reply back with the language different from you questioned one. So we have to repond via binary reply. Currently, aiohttp supports both Response and HTTPException instances to generate responses. Once exceptions are cannot carry binary data, they becomes quite off the road and we have to stay with Response and use return instead of raise.

> May be we can push this story to the end and leave only one way to generate response from a server?

This is interesting.

Adding `body` support to base `HTTPException` is an easy change.
It naturally leads to a problem: support really *dual* API in exceptions: one for text and another for binaries (splitting `web.Response` into text and binary classes also make sense but let's keep a focus on exceptions).
Dual API can be provided for two exception classes for every HTTP status code: one for text content and another one for binary. Two separate exception hierarchies undesirable: I want to write `except HTTPForbidden` in application code, `except (HTTPForbiddenText, HTTPForbiddenBinary)` doesn't work.
We can keep the hierarchy as is but addind binary and text subclasses to any leaf class.
For example existing `HTPPForbidden` becomes an abstract class with two instantiable terminal children: `HTTPForbiddenText` and `HTTPForbiddenBinary`.
It makes everything overcomplicated for my taste.

What is http exception and why they are used by code, that's the question?
If you consider it as a *flow control statement* which is used to alter the normal execution -- that's simplifies things. Even *text* is superfluous for pure control structure. HTTP exceptions are not for rendering the content but for fast jump out of the normal handling if, e.g., a user has no requested access rights.
Any complex program should have top-level error-page handler to return a proper `web.Response` wtih corresponding status code and (maybe binary) content.
The current design supports this approach but simplifies things a little for text contents. Binaries take more work but still possible.
Another minor problem with binary HTTP exceptions is printability and human readability.
Now `HTTPException` has no `__repr__()` but the method can be added and makes sense.
For current implementation `__repr__` can be constructed from status code, provided headers and text message. Binary data has no human representation by definition, using `repr(binary_body)` is useless usually. Initially, I used to add `HTTPException.build_response()` virtual method to construct `web.Response` from it but gave up the idea because the overriding was not needed for aiohttp code itself.

Summary: exceptions are neutral from text/binary perspective for exception raiser and catcher. The only place where the separation matters are building HTTP response from a given exception. API is bent to build text exceptions easier than binaries, text responses are much more common in HTTP world.

コントリビューションガイド

コントリビューションガイドを開く

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。