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

Unhandled BaseException cause upstrream connection disconnected sliently #2923

Closed
min-tian-thomas opened this issue Jan 20, 2023 · 2 comments

Comments

@min-tian-thomas
Copy link

min-tian-thomas commented Jan 20, 2023

Context
If Gunicron is configured with keepalive != 0, then base_async.py will handle it with keepalive loop.

                    # keepalive loop
                    proxy_protocol_info = {}
                    while True:
                        req = None
                        with self.timeout_ctx():
                            req = next(parser)
                        if not req:
                            break
                        if req.proxy_protocol_info:
                            proxy_protocol_info = req.proxy_protocol_info
                        else:
                            req.proxy_protocol_info = proxy_protocol_info
                        self.handle_request(listener_name, req, client, addr)

Here when using worker type as: worker_class = "gevent", then gevent.py defined as:

    def timeout_ctx(self):
        return gevent.Timeout(self.cfg.keepalive, False)

This logic will throw out gevent.Timeout type when timeout and our logic cannot handle that in all different kinds of exception logic until getting to the final branch: https://github.com/benoitc/gunicorn/blob/master/gunicorn/workers/base_async.py#L87
Because:

class Timeout(BaseException):

Impact
This will be a problem as a connection between the gateway (eg: Nginx) and Gunicorn will be suddenly closed without response and Nginx will expect the service was died and return back clients with error 502.

From Nginx you will see something that is not reasonable like:

2023/01/20 13:46:02 [error] 1027#1027: *209 upstream prematurely closed connection while reading response header from upstream, client: 127.0.0.1, server: flask_server.com, request: "GET /sleep?time=1 HTTP/1.1", upstream: "http://127.0.0.1:8000/sleep?time=1", host: "flask_server.com"

The error can also be triggered when throwing out any other exception that is not inherited from Exception but BaseException from user logic in Flask app.

@benoitc
Copy link
Owner

benoitc commented Dec 7, 2023

thanks for the patch even if you just closed it. I will apply it so we can catcg all exceptions under python 3.

@benoitc benoitc closed this as completed Dec 7, 2023
benoitc added a commit that referenced this issue Dec 7, 2023
ensure we can catch  correctly  exceptions based on BaseException.

Note: patch was origninally proposed by the pr #2923, but original
author closed it.

Fix #2923
@min-tian-thomas
Copy link
Author

thanks for the patch even if you just closed it. I will apply it so we can catcg all exceptions under python 3.

Welcome, I deleted my forked repo without expecting this PR to be closed there. Glad to see this change has been merged!

jeffesp pushed a commit to datarobot-forks/gunicorn-drfork that referenced this issue Oct 2, 2024
ensure we can catch  correctly  exceptions based on BaseException.

Note: patch was origninally proposed by the pr benoitc#2923, but original
author closed it.

Fix benoitc#2923
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

2 participants