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

Access log logs wrong status on file responses #347

Open
gaborbernat opened this issue Jul 8, 2024 · 5 comments
Open

Access log logs wrong status on file responses #347

gaborbernat opened this issue Jul 8, 2024 · 5 comments
Labels
asgi Issue related to ASGI protocol bug Something isn't working rsgi Issue related to RSGI protocol

Comments

@gaborbernat
Copy link

gaborbernat commented Jul 8, 2024

I noticed that 200s are logged as 500. Modified https://github.com/emmett-framework/granian/blob/master/granian/asgi.py#L109 with:

def _callback_wrapper(callback, scope_opts, state, access_log_fmt=None):
    root_url_path = scope_opts.get("url_path_prefix") or ""

    def _runner(scope, proto):
        scope.update(root_path=root_url_path, state=state.copy())
        return callback(scope, proto.receive, proto.send)

    async def _http_logger(scope, proto):
        t = time.time()
        try:
            rv = await _runner(scope, proto)
        finally:
            interval = 0.0001
            for i in range(5):
                print(f"{interval * i} code: {proto.sent_response_code}")
                time.sleep(interval)
            access_log(t, scope, proto.sent_response_code)
        return rv

    def _logger(scope, proto):
        if scope["type"] == "http":
            return _http_logger(scope, proto)
        return _runner(scope, proto)

    access_log = _build_access_logger(access_log_fmt)
    wrapper = _logger if access_log_fmt else _runner
    wraps(callback)(wrapper)

    return wrapper

Output:

2024-07-08 09:39:14.214-0700 13785 INFO _granian.asgi.serve Started worker-2
2024-07-08 09:39:14.214-0700 13785 INFO _granian.asgi.serve Started worker-2 runtime-1
2024-07-08 09:39:14.280-0700 13784 INFO _granian.asgi.serve Started worker-1
2024-07-08 09:39:14.280-0700 13784 INFO _granian.asgi.serve Started worker-1 runtime-1

0.0 code: 500
0.0001 code: 500
0.0002 code: 200
0.00030000000000000003 code: 200
0.0004 code: 200

2024-07-08 09:39:17.831-0700 13784 INFO granian.access 127.0.0.1 - "GET /favicon.ico HTTP/1.1" 200 20.887
0.0 code: 500
0.0001 code: 500
0.0002 code: 200
0.00030000000000000003 code: 200
0.0004 code: 200

2024-07-08 09:39:18.499-0700 13784 INFO granian.access 127.0.0.1 - "GET /favicon.ico HTTP/1.1" 200 3.204
0.0 code: 500
0.0001 code: 500
0.0002 code: 200
0.00030000000000000003 code: 200
0.0004 code: 200

As you can see, the access log is logged before the response is sent, and I assume 500 is the default value.

The response type in question is a FileResponse from starlette. When I remove my for loop, the status code logged is always 500.

@gi0baro gi0baro added bug Something isn't working asgi Issue related to ASGI protocol rsgi Issue related to RSGI protocol labels Jul 8, 2024
@gi0baro gi0baro changed the title Access log logs bad status in ASGI Access log logs wrong status on file responses Jul 8, 2024
@gi0baro gi0baro added this to the 1.5 milestone Jul 8, 2024
@gi0baro
Copy link
Member

gi0baro commented Jul 8, 2024

This is confirmed, and it happens on ASGI pathsend responses and RSGI response_file responses.
The issues is caused by the fact Granian won't block the Python application for file opening and reading; and just making the app waiting for that looks very sub-optimal to me.
Will think about a solution for this in the following days.

@gaborbernat
Copy link
Author

Are you going to EuroPython by any chance? :D

@gi0baro
Copy link
Member

gi0baro commented Jul 8, 2024

Are you going to EuroPython by any chance? :D

Nope, unfortunately this year I won't be able to attend :(

@gaborbernat
Copy link
Author

This is confirmed, and it happens on ASGI pathsend responses and RSGI response_file responses. The issues is caused by the fact Granian won't block the Python application for file opening and reading; and just making the app waiting for that looks very sub-optimal to me. Will think about a solution for this in the following days.

Hardcode response to 200 if the file exists? 😆 Though would not handle not changed responses...

@gi0baro
Copy link
Member

gi0baro commented Jul 9, 2024

So, I pushed 72a9a21 that won't fix the actual issue, but at least prevents fake 500 status codes in access logs.
This means next revision will report 200 even if Granian is not able to open the file (and thus the real response code will be 404).

I'll keep this opened until I find a proper fix.

@gi0baro gi0baro removed this from the 1.5 milestone Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
asgi Issue related to ASGI protocol bug Something isn't working rsgi Issue related to RSGI protocol
Projects
None yet
Development

No branches or pull requests

2 participants