Skip to content

I found a problem in the rest_framework/views.py file #6404

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

Closed
halohsu opened this issue Jan 13, 2019 · 5 comments
Closed

I found a problem in the rest_framework/views.py file #6404

halohsu opened this issue Jan 13, 2019 · 5 comments

Comments

@halohsu
Copy link

halohsu commented Jan 13, 2019

Hello, good evening. I found a problem in the rest_framework/views.py file. See the source code snippet below:

def raise_uncaught_exception(self, exc):
    if settings.DEBUG:
         request = self.request
         renderer_format = getattr(request.accepted_renderer, 'format')
         use_plaintext_traceback = renderer_format not in ('html', 'api', 'admin')
         request.force_plaintext_errors(use_plaintext_traceback)
    raise

What program exception do you want to throw? I guess uncaught exception.
I uninstall it and reinstall it like this. My installation process is as follows:

(venv) D:\workspace\pycharm\Mx_Shopping>pip install djangorestframework
Looking in indexes: http://mirrors.aliyun.com/pypi/simple
Collecting djangorestframework
  Downloading http://mirrors.aliyun.com/pypi/packages/99/0b/d37a5a96c5d301e23adcabcc2f3fa659fb34e6308590f95ebb50cdbe98a1/djangorestframework-3.9.0-py2.py3-none-any
.whl (924kB)
    100% |████████████████████████████████| 931kB 6.6MB/s
Installing collected packages: djangorestframework
Successfully installed djangorestframework-3.9.0
You are using pip version 10.0.1, however version 18.1 is available.
You should consider upgrading via the 'python -m pip install --upgrade pip' command.

2019-01-13_201226

@tomchristie
Copy link
Member

raise on its own will reraise the latest exception

@rpkilby
Copy link
Member

rpkilby commented Jan 30, 2019

A few thoughts:

  • Would raise and raise exc have different behavior?
  • If so, should we document why we're using raise instead of raise exc?
  • If not, why not favor raise exc over raise, which is more explicit?

@tomchristie
Copy link
Member

Good question, actually!

@halohsu
Copy link
Author

halohsu commented Jan 31, 2019

非常感谢“encode”成员再次关注这个问题。
Many thanks to the "encode" members for paying attention to this issue again.
我认为应该优先使用 raise exc
I think it should be preferred to use raise exc.
这会让使用“drf”构建的应用程序更加具有可读性。
This will make applications built with "drf" more readable.
如果仅仅使用 raise` `我认为是不负责任的。 If I only use raise`` I think it is irresponsible.
请试想一下,如果一个程序员使用"drf"构建应用,在捕获异常时使用以下哪种方式更加优雅呢?
Imagine if a programmer uses "drf" to build an application, which of the following is more elegant when catching exceptions?
1.使用"raise":
Use "raise"

try:
	drf code...
except Exception:
	write error log...

2.使用"raise exc":
Use "raise exc"

try:
	drf code...
except (OSError, IOError):
	write panic log...
except CustomError:
	write log...

我认为第二种方式更加优雅和清晰。
I think the second way is more elegant and clear.
您可能认为在这里没有必要,但是这是个好的习惯,因为一些开发者习惯按住Ctrl键随时查看框架源代码。
You might think it's not necessary here, but it's a good habit because some developers are used to holding down the Ctrl key to view the framework source code at any time.

@rpkilby
Copy link
Member

rpkilby commented Feb 2, 2019

@tomchristie, had a bit of a think and raise exc seems like The Correct Thing To Do TM.

As an unlikely example, a user might override handle_exception, and generate a new exception object from the original. This exception would be passed, not raised, since we're already in the middle of the exception handler. However, a bare raise would raise the original exception, not the one the user passed. Compare the following (the latter is more "correct"):

>>> def handle(exc):
...     raise
... 
>>> try:
...     raise Exception('!!!')
... except Exception as exc:
...     handle(TypeError('???'))
... 
Traceback (most recent call last):
  File "<stdin>", line 4, in <module>
  File "<stdin>", line 2, in <module>
Exception: !!!
>>> def handle(exc):
...     raise exc
... 
>>> try:
...     raise Exception('!!!')
... except Exception as exc:
...     raise TypeError('???')
... 
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
Exception: !!!

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 4, in <module>
TypeError: ???

Either way, I can't think of a reason to keep a bare raise.

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

No branches or pull requests

3 participants