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.

Sign up for GitHub

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

Jump to bottom

Unhandled BaseException cause upstrream connection disconnected sliently #2923

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

Unhandled BaseException cause upstrream connection disconnected sliently #2923

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

Comments

@min-tian-thomas
Copy link

min-tian-thomas commented Jan 20, 2023 •
edited
Loading

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.
@min-tian-thomas min-tian-thomas mentioned this issue Jan 20, 2023
Closed
@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
@benoitc
let's exception not bubble
4023228 
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!

@pajod pajod mentioned this issue Aug 14, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@benoitc @min-tian-thomas