basecamp / kamal-proxy

Lightweight proxy server for Kamal
https://kamal-deploy.org/
MIT License
751 stars 31 forks source link

Cancelled requests returning status 502 #59

Closed Rohland closed 1 week ago

Rohland commented 4 weeks ago

Hi Team,

Should cancelled client http requests be logging a status 502? I believe Nginx uses 499. There is probably an argument for returning 204 (no content). 502 is a bit ambiguous and is triggering alerts when it shouldn't. Just wondering whether 502 is the right status code in this context.

Background: We're seeing this in our kamal proxy logs (redacted information is xxx):

{
    "time": "2024-10-29T08:52:38.632232862Z",
    "level": "INFO",
    "msg": "Request",
    "host": "xxx",
    "port": 80,
    "path": "/groups/search",
    "request_id": "fd2ba705-06d6-42bc-af56-e052b1233b44",
    "status": 502,
    "service": "xxx",
    "target": "31eab6575fc4:3000",
    "duration": 171945507,
    "method": "POST",
    "req_content_length": 128,
    "req_content_type": "application/x-www-form-urlencoded;charset=UTF-8",
    "resp_content_length": 6214,
    "resp_content_type": "text/html; charset=utf-8",
    "client_addr": "xxx",
    "client_port": "6886",
    "remote_addr": "xxx, xxx",
    "user_agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/130.0.0.0 Safari/537.36",
    "proto": "HTTP/1.1",
    "scheme": "http",
    "query": "",
    "req_cf_connecting_ip": "xxx"
}

With a corresponding message in the kamal-proxy like this:

{
    "time": "2024-10-29T08:52:38.631982324Z",
    "level": "ERROR",
    "msg": "Error while proxying",
    "target": "31eab6575fc4:3000",
    "path": "/groups/search",
    "error": "context canceled"
}
kevinmcconnell commented 1 week ago

Hi @Rohland, thanks for bringing this up. I agree the 502 is misleading in this case; we should be logging it differently. 499 seems like a good choice since it's unambiguous and is used elsewhere. I'll take a look at changing this.

kevinmcconnell commented 1 week ago

Fix for this will be in the next release.