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

#930 The WebSocket is in an invalid state ('Aborted') for this operation #1030 #1576

Closed
wants to merge 1 commit into from

Conversation

OnSive
Copy link

@OnSive OnSive commented May 9, 2022

Reopened PR: #1030

tdejong-tools4ever commented on Sep 30, 2019

@raman-m raman-m added the in progress Someone is working on the issue. Could be someone on the team or off. label May 11, 2023
@raman-m raman-m self-requested a review May 11, 2023 15:25
@raman-m raman-m changed the title reopened pr #1030 #930 #1030 The WebSocket is in an invalid state ('Aborted') for this operation May 11, 2023
@raman-m raman-m changed the title #930 #1030 The WebSocket is in an invalid state ('Aborted') for this operation #930 The WebSocket is in an invalid state ('Aborted') for this operation #1030 May 11, 2023
Copy link
Member

@raman-m raman-m left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do NOT see any fixes!
I see extra code only with extra post-action.

Could you add a couple of unit tests please?
Also, could you write at least one acceptance test please?

Comment on lines 50 to +54
await destination.CloseOutputAsync(WebSocketCloseStatus.EndpointUnavailable, null, cancellationToken);
if (destination.State == WebSocketState.Open || destination.State == WebSocketState.CloseReceived)
{
await destination.CloseOutputAsync(WebSocketCloseStatus.EndpointUnavailable, null, cancellationToken);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simple logical challenge:
How many times will the action be applied here?
Once or twice?

The action is CloseOutputAsync.

Comment on lines 61 to +65
await destination.CloseOutputAsync(WebSocketCloseStatus.EndpointUnavailable, null, cancellationToken);
if (destination.State == WebSocketState.Open || destination.State == WebSocketState.CloseReceived)
{
await destination.CloseOutputAsync(WebSocketCloseStatus.EndpointUnavailable, null, cancellationToken);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you apply the same action twice?

The action is CloseOutputAsync.

@OnSive
Copy link
Author

OnSive commented May 12, 2023

I'm closing this PR as I can't remember what it was actually about and the original author is actively communicating on #1030

@OnSive OnSive closed this May 12, 2023
@raman-m raman-m added bug Identified as a potential bug and removed in progress Someone is working on the issue. Could be someone on the team or off. labels May 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Identified as a potential bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The WebSocket is in an invalid state (Aborted) for this operation. Valid states are: Open, CloseReceived
2 participants