-
Notifications
You must be signed in to change notification settings - Fork 268
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
Twisted 24.10.0 incompatibility #535
Comments
@karolyi Thanks for the report! Will need to dig in. If you're able to investigate, super. |
Looks like tests pass with 24.3 but fail with 24.7. https://github.com/twisted/twisted/releases/tag/twisted-24.7.0 |
Hmmm. But my test project is working with both 24.7 and 24.10, so it's not immediately obvious where the problem comes up. Will have to dig. Are you able to provide a demo that triggers the error @karolyi ? |
It's a pretty huge project and I'm spent right now (been through a huge dev process, taking a lot outta me), but from what I can see, it happens on websocket connections. That's all I can tell now, not sure it helps. I'm pretty busy in the upcoming days (reached a huge milestone and have to get the whole thing into production), so for now I went back to 24.7.0 which works. I might put something together later on to reproduce the bug, if you don't manage to do it until then. |
@karolyi OK, no stress. Thanks for the heads up. I will try and pin it down. If any more info comes your way, please do add it (but not an issue if you're busy). Take care. |
🤝 |
OK, failures I was seeing were an artefact. (I had a checkout on a branch prior to #525 🤦) |
There used to be a conditional check at the error location 0b37e80#diff-1ed03cbfb8907b3705de148e4a604b28290b730db6df250bac1081215be50301R114 ... which explains the comment. Need to see when that disappeared. (Bets now that it was me that removed it 😅) |
From the comments on b72349d:
|
I have this error on production. Are there any workarounds? |
I'm onto something here, please hold on. |
So the issue clearly can be linked to running daphne (and not django's
This way I can reproduce the bug locally too. But in order to test this, you'll need a minimally configured nginx (which I luckily have) that will listen on the actual http port and forward the messages to the unix socket. @hagen00 your best course of action is the same as mine, going back to 24.7.0 until this gets fixed. |
Full traceback:
Fun fact, it does happen with normal HTTP requests too, but at least those get served. On a second look (and looking at the full traceback), this seems more like a twisted bug, not a daphne one. |
I'm opening a bugreport in twisted for this. |
Yes, thanks a lot! Pinning it like this in requirements.txt fixed it for me
|
Good work @karolyi! 🎁 👀 |
Thanks, good things happen when you get a good night's sleep and are through the morning coffee ritual, sacrificing a little time for this before the actual paid work begins. :) What's interesting to me is that the exception line coming from production (see the first comment) differs from that of my dev env that has the bug reproduced. Nonetheless, it seems this is a bug in twisted. Let's give some time to them to get this figured out. |
Hi. It looks like daphne/daphne/http_protocol.py Lines 149 to 150 in 06afd9b
At Twisted we put a lot of effort to keep backward compatibilty, but this is only for the public API. If there is any need to use a private API, please create a GitHub issue to If you need to use private API, we highly recommend to setup automated tests using the There is an ongoing effort to improve the performance of Twisted and this will involve doing some internal private changes. |
Now, there is also the other error reported by László #535 (comment) and this looks like a Twisted bug
|
@karolyi I don't suppose you can bisect the issue to a particular commit in twisted can you? |
Sorry but no. I'm not in the position right now where I can put in the effort for it. Hope I could help with attributing the issue. |
OK, no problem. I still need to get a reproduce working locally, then I can run git bisect on twisted. |
Hello,
Twisted 24.10.0 is officially out: https://github.com/twisted/twisted/releases/tag/twisted-24.10.0
Daphne seems to be incompatible with it. I'm running 4.1.2. The traceback is:
The text was updated successfully, but these errors were encountered: