Skip to content
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

[Java Client] HTTP errors during WebSocket handshake not surfaced as HttpRequestExceptions #47597

Open
1 task done
codylund opened this issue Apr 6, 2023 · 0 comments
Open
1 task done
Labels
area-signalr Includes: SignalR clients and servers

Comments

@codylund
Copy link

codylund commented Apr 6, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

If an HTTP response for the opening SignalR negotiate call contains an unexpected response code, the SignalR Java client library will convert the error to an HttpRequestException for use by consumers of the library. However, if negotiation is skipped and a similar error occurs during the opening WebSocket handshake, the library does not return a useful exception. Instead, OkHttpWebSocketWrapper.SignalRWebSocketListener.onFailure() ignores the Response returned by OkHttp and wraps the provided Throwable in a RuntimeException.

This makes certain handleable errors, like 401, especially difficult to detect. I believe the only recourse today is to parse the error message string on the underlying Throwable returned by OkHttp.

Expected Behavior

When negotiation is skipped, SignalR should return HttpRequestExceptions for the opening HTTP handshake in the WebSocket protocol, so we can handle the error codes (like 401) appropriately.

Steps To Reproduce

  1. Set up a basic SignalR Java client and SignalR service that supports authentication.
  2. Configure the SignalR client to use WebSocket transport and skip negotiation.
  3. Provide a bad token for the connection.

Expected: SignalR Java client receives an HttpRequestException with a 401 code.
Actual: SignalR Java client receives a generic RuntimeException.

Exceptions (if any)

No response

.NET Version

No response

Anything else?

No response

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-signalr Includes: SignalR clients and servers label Apr 6, 2023
@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-signalr Includes: SignalR clients and servers
Projects
None yet
Development

No branches or pull requests

2 participants