Closed alex-anthropic closed 6 months ago
@pgjones we just discovered the hard way that you reverted this... did you come up with the better solution you were looking for? Or could we un-revert it until you have a chance to replace with the proper fix?
I thought I had, how did it crash?
On Tue, 24 Sept 2024, 00:18 Nathaniel J. Smith, @.***> wrote:
@pgjones https://github.com/pgjones we just discovered the hard way that you reverted this... did you come up with the better solution you were looking for? Or could we un-revert it until you have a chance to replace with the proper fix?
— Reply to this email directly, view it on GitHub https://github.com/pgjones/hypercorn/pull/228#issuecomment-2369749923, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKTW6V6ZXTSZ5MRDYJCE4LZYCOSPAVCNFSM6AAAAABHPJSKQCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNRZG42DSOJSGM . You are receiving this because you were mentioned.Message ID: @.***>
@pgjones I'd have to dig up details, but it involved BusyResourceError: another task is currently sending data on this SocketStream
taking down the entire server.
Can you point to the better solution you did? Looking at main
right now I don't see any last-ditch exception handlers between hypercorn.trio.tcp_server.TCPServer.run
and the serve_listeners
in hypercorn.trio.run.worker_serve
, that could stop an error on one request from taking down the entire server.
Partial mitigation for https://github.com/pgjones/hypercorn/issues/225
This doesn't fix the underlying
AttributeError: 'WSStream' object has no attribute 'connection'
but it does allow the server to stay up and healthy when it encounters this bug.