benoitc / gunicorn

gunicorn 'Green Unicorn' is a WSGI HTTP Server for UNIX, fast clients and sleepy applications.
http://www.gunicorn.org
Other
9.86k stars 1.75k forks source link

revert: 4023228 ("let's exception not bubble") #3275

Open pajod opened 3 months ago

pajod commented 3 months ago

The report in https://github.com/benoitc/gunicorn/issues/2923 may have been not caused by a bug in Gunicorn or gevent in the first place. In any case, going from "this particular exception" to "each and every BaseException" was way excessive and is a likely explanation for https://github.com/benoitc/gunicorn/issues/3207

Context:

Suggested changes:

Unfinished thoughts:

pajod commented 2 months ago

@benoitc Recommend you test this in conjunction with the two patches by sylt. With those, it is no longer a "simple" revert, but gets us closer to confirming the spurious tracebacks reported in #3207 are dealt with.

(hint: there is documentation on what exactly my integration tests currently encompass here: https://github.com/pajod/gunicorn/pull/3)

benoitc commented 2 months ago

@pajod PR must be provided over the master and don't assume that other PR may be added. There is no guarantee a separate PR will be acccepted, so PR will be enough by itself.

This PR sounds OK by itself. What could miss?

pajod commented 2 months ago

@benoitc This PR fits on current master. It is OK to merge by itself.