Websocket doesn't work when using version parameter

[danieldaeschle]

Describe the bug
When defining a websocket handler using version=1 a exceptions will be thrown.

Code snippet
Relevant source code, make sure to remove what is not necessary.

@app.websocket("/changes", version=1)

Expected behavior
It should work like without version=1

Environment (please complete the following information):

• OS: Fedora 31
• Sanic: 19.12.2

[danieldaeschle]

I found out, that the websocket path is still /changes without /v1 as prefix.
It's a routing issue.

[danieldaeschle]

version is missing here: https://github.com/huge-success/sanic/blob/master/sanic/app.py#L519

[ahopkins]

@danieldaeschle Would you be able to submit a PR and perhaps a unit test?

[danieldaeschle]

I didn't found the root case yet. I'm not familiar with the sanic code. Do you have a hint for me? Otherwise i have to dig deeper an try to submit one. Adam Hopkins <[email protected]> schrieb am Mo., 6. Jan. 2020, 21:12:

…

@danieldaeschle <https://github.com/danieldaeschle> Would you be able to submit a PR and perhaps a unit test? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1757>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHKTWHDQTETSXJJN5FV3OQDQ4OGCBANCNFSM4KC3JRTA> .

[ahopkins]

I did a quick look at the code you linked to on my phone. I'm fairly certain that you pinpointed the right place. All you should need to do is add the version kwarg to the method and then to the call that you linked. If you want to just do that much I can then take a look and give you some more direction on where and how to add the test.

[danieldaeschle]

I already tried adding that kwarg. But that wasn't enough to work. Adam Hopkins <[email protected]> schrieb am Di., 7. Jan. 2020, 07:14:

…

I did a quick look at the code you linked to on my phone. I'm fairly certain that you pinpointed the right place. All you should need to do is add the version kwarg to the method and then to the call that you linked. If you want to just do that much I can then take a look and give you some more direction on where and how to add the test. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1757>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHKTWHFJAN3GKFAO2DXRF73Q4QMUFANCNFSM4KC3JRTA> .

[ahopkins]

OK. Thanks for taking a shot. I can take this one in conjunction with upgrading websockets dependency.

[danieldaeschle]

Thanks! Is it possible to get this fix soon? Am Di., 7. Jan. 2020 um 18:04 Uhr schrieb Adam Hopkins < [email protected]>:

…

OK. Thanks for taking a shot. I can take this one in conjunction with upgrading websockets dependency. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1757>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHKTWHF53DZANYMZSMP5MULQ4SYZTANCNFSM4KC3JRTA> .

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If this is incorrect, please respond with an update. Thank you for your contributions.