Raising a 412 on a Sanic handler causes a wait period and maybe a ReadTimeout exception on clients

[javierdialpad]

Describe the bug

When raising a 412, clients are left waiting to read until a timeout is triggered and a ReadTimeout exception may occur.

Code snippet

The issue can be reproduced with code for an app as minimal as:

# Server Side

from sanic import Sanic
from sanic import exceptions

app = Sanic("MyHelloWorldApp")


@app.get('/hello')
async def hello_world(request, *args, **kwargs):
  raise exceptions.SanicException('Failed Precondition', status_code=412)


if __name__ == '__main__':
  app.run()

The client can be Postman, bash curl, or even a Python fetch:

# Client Side

import asyncio
import time

import httpx


async def async_fetch():
  async with httpx.AsyncClient() as client:
    return await client.get('http://localhost:8000/hello')


if __name__ == '__main__':
  now = time.time()
  try:
    resp = asyncio.get_event_loop().run_until_complete(async_fetch())
  except Exception as e:
    print('Exception:', type(e))
  else:
    print('Response:', resp)
  finally:
    print('Time:', time.time() - now)

The above client code will print:

Exception: <class 'httpx.ReadTimeout'>
Time: 5.057372808456421

Expected behavior

The client should quickly receive the 412 response.

Environment (please complete the following information):

• OS: macOS Monterey with M1 chip
• Sanic Version: 22.9.0

Additional context

What I think is happening is that Sanic strips the headers on 412 responses, including the content-type header. However, and contrary to what happens for a 304 code, Sanic will still send the 412 response body to the client, which causes the latter to wait for more data, without knowing the actual size of the response, until it times out.

[ahopkins]

I can confirm this. Looks like @Tronic had an intention to come back here at some point:

# TODO: Make a blacklist set of header names and then filter with that

[Tronic]

@ahopkins Any idea on why exactly Sanic strips headers on conditional responses (304, 412)? Shouldn't the app just not set the entity headers if not needed? Looks like the function being called does what that TODO says, anyway, but it wasn't originally my work.

[ahopkins]

Nope. I want to dig into that and figure out where/when it was added for context. Also I want to dig into the RFC because what I understand there shouldn't be any header removal for 412. I didn't see anything that would indicate a restriction in content or headers allowed. But I only had time to take a cursory look at the earlier RFC and nine of the revisions.

[ahopkins]

Fix is coming in the next release.

[ChihweiLHBird]

Hi @ahopkins, FYI: content-length was removed for 412 responses since https://github.com/sanic-org/sanic/pull/1127

Other than the PR description itself, there was not much discussion about the header removal for 412 responses, nor were any of the referenced materials/RFC explaining why the content-length header shouldn't be in 412 responses.

I also asked chatGPT and got:

When responding with an HTTP 412 status code (Precondition Failed), it is allowed to include an error message in the response body.

I would assume that was a mistake and believe we can safely remove it.

[ChihweiLHBird]

Fixed in https://github.com/sanic-org/sanic/pull/2824 and https://github.com/sanic-org/sanic/pull/2962.