Skip to content

More explicit error message when redirected to a non websocket uri #1632

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

anthony-moreau
Copy link

When using the asyncio client, the error message and exception raised is the same if the user provides an incorrect uri or if the server respondes with a 302 found with a location header pointing to an incorrect uri.

This is quite confusing as it took me some time and debugging of the websocket library to figure out that the problem wasn't directly in my code but rather in the response of the server.

The current behavior isn't a bug as the uri was indeed a webpage and not a websocket so an exception in this scenario is normal. However a more explicit error message would help users understand the exact issue better.

As a result I'm suggesting changes to catch the InvalidUri exception in this scenario and modify the error message to make it more explicit. I also added a test to reflect theses changes.

@anthony-moreau
Copy link
Author

Hey, did you have time to take a look at this? I fixed the lint error in the test file.

@aaugustin
Copy link
Member

Back when you submitted it, I noticed that the error message was getting heavy and repetitive. However, I didn't have an obvious idea to do better.

Options include:

  • introducing an InvalidRedirectURI exception type;
  • refactoring InvalidURI to customize the message — must be backwards compatible;
  • other?

I haven't worked on this project in the past few weeks. I'm likely to have more time during the summer.

@anthony-moreau
Copy link
Author

anthony-moreau commented Jun 29, 2025

I took some time to look again at my changes and you're right. I didn't read the definition of the InvalidUri exception so what I did wasn't optimal.

I decided to add a new InvalidRedirectURI exception subclassing InvalidUri with a different message as it looks to me like the simplest solution to the problem.

Feel free to take a look at it when you have the time to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants