Skip to content

Commit

Permalink
Use WebSocketConnectionClosed for operations after WebSocket close (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
brianquinlan authored Apr 25, 2024
1 parent 9780081 commit e459e5c
Show file tree
Hide file tree
Showing 8 changed files with 58 additions and 19 deletions.
3 changes: 3 additions & 0 deletions pkgs/cupertino_http/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
## 1.4.1-wip

* Upgrade to `package:ffigen` 11.0.0.
* Bring `WebSocket` behavior in line with the documentation by throwing
`WebSocketConnectionClosed` rather `StateError` when attempting to send
data to or close an already closed `CupertinoWebSocket`.
* Update minimum supported iOS/macOS versions to be in sync with the minimum
(best effort) supported for Flutter: iOS 12, macOS 10.14

Expand Down
8 changes: 5 additions & 3 deletions pkgs/cupertino_http/lib/src/cupertino_web_socket.dart
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ class CupertinoWebSocket implements WebSocket {
/// Handle an incoming message from the peer and schedule receiving the next
/// message.
void _handleMessage(URLSessionWebSocketMessage value) {
if (_events.isClosed) return;

late WebSocketEvent event;
switch (value.type) {
case URLSessionWebSocketMessageType.urlSessionWebSocketMessageTypeString:
Expand Down Expand Up @@ -160,7 +162,7 @@ class CupertinoWebSocket implements WebSocket {
@override
void sendBytes(Uint8List b) {
if (_events.isClosed) {
throw StateError('WebSocket is closed');
throw WebSocketConnectionClosed();
}
_task
.sendMessage(URLSessionWebSocketMessage.fromData(Data.fromList(b)))
Expand All @@ -170,7 +172,7 @@ class CupertinoWebSocket implements WebSocket {
@override
void sendText(String s) {
if (_events.isClosed) {
throw StateError('WebSocket is closed');
throw WebSocketConnectionClosed();
}
_task
.sendMessage(URLSessionWebSocketMessage.fromString(s))
Expand All @@ -180,7 +182,7 @@ class CupertinoWebSocket implements WebSocket {
@override
Future<void> close([int? code, String? reason]) async {
if (_events.isClosed) {
throw StateError('WebSocket is closed');
throw WebSocketConnectionClosed();
}

if (code != null) {
Expand Down
6 changes: 6 additions & 0 deletions pkgs/web_socket/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
## 0.1.3

- Bring the behavior in line with the documentation by throwing
`WebSocketConnectionClosed` rather `StateError` when attempting to send
data to or close an already closed `WebSocket`.

## 0.1.2

- Fix a `StateError` in `IOWebSocket` when data is received from the peer
Expand Down
6 changes: 3 additions & 3 deletions pkgs/web_socket/lib/src/browser_web_socket.dart
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ class BrowserWebSocket implements WebSocket {
@override
void sendBytes(Uint8List b) {
if (_events.isClosed) {
throw StateError('WebSocket is closed');
throw WebSocketConnectionClosed();
}
// Silently discards the data if the connection is closed.
_webSocket.send(b.jsify()!);
Expand All @@ -113,7 +113,7 @@ class BrowserWebSocket implements WebSocket {
@override
void sendText(String s) {
if (_events.isClosed) {
throw StateError('WebSocket is closed');
throw WebSocketConnectionClosed();
}
// Silently discards the data if the connection is closed.
_webSocket.send(s.jsify()!);
Expand All @@ -122,7 +122,7 @@ class BrowserWebSocket implements WebSocket {
@override
Future<void> close([int? code, String? reason]) async {
if (_events.isClosed) {
throw StateError('WebSocket is closed');
throw WebSocketConnectionClosed();
}

checkCloseCode(code);
Expand Down
6 changes: 3 additions & 3 deletions pkgs/web_socket/lib/src/io_web_socket.dart
Original file line number Diff line number Diff line change
Expand Up @@ -86,23 +86,23 @@ class IOWebSocket implements WebSocket {
@override
void sendBytes(Uint8List b) {
if (_events.isClosed) {
throw StateError('WebSocket is closed');
throw WebSocketConnectionClosed();
}
_webSocket.add(b);
}

@override
void sendText(String s) {
if (_events.isClosed) {
throw StateError('WebSocket is closed');
throw WebSocketConnectionClosed();
}
_webSocket.add(s);
}

@override
Future<void> close([int? code, String? reason]) async {
if (_events.isClosed) {
throw StateError('WebSocket is closed');
throw WebSocketConnectionClosed();
}

checkCloseCode(code);
Expand Down
2 changes: 1 addition & 1 deletion pkgs/web_socket/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ description: >-
Any easy-to-use library for communicating with WebSockets
that has multiple implementations.
repository: https://github.com/dart-lang/http/tree/master/pkgs/web_socket
version: 0.1.2
version: 0.1.3

environment:
sdk: ^3.3.0
Expand Down
26 changes: 20 additions & 6 deletions pkgs/web_socket_conformance_tests/lib/src/close_local_tests.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'dart:typed_data';

import 'package:async/async.dart';
import 'package:stream_channel/stream_channel.dart';
import 'package:test/test.dart';
Expand Down Expand Up @@ -119,13 +121,25 @@ void testCloseLocal(

await expectLater(
() async => await channel.close(3001, 'Client initiated closure'),
throwsStateError);
final closeCode = await httpServerQueue.next as int?;
final closeReason = await httpServerQueue.next as String?;
throwsA(isA<WebSocketConnectionClosed>()));
});

expect(closeCode, 3000);
expect(closeReason, 'Client initiated closure');
expect(await channel.events.isEmpty, true);
test('sendBytes after close', () async {
final channel = await channelFactory(uri);

await channel.close(3000, 'Client initiated closure');

expect(() => channel.sendBytes(Uint8List(10)),
throwsA(isA<WebSocketConnectionClosed>()));
});

test('sendText after close', () async {
final channel = await channelFactory(uri);

await channel.close(3000, 'Client initiated closure');

expect(() => channel.sendText('Hello World'),
throwsA(isA<WebSocketConnectionClosed>()));
});
});
}
20 changes: 17 additions & 3 deletions pkgs/web_socket_conformance_tests/lib/src/close_remote_tests.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.

import 'dart:typed_data';

import 'package:async/async.dart';
import 'package:stream_channel/stream_channel.dart';
import 'package:test/test.dart';
Expand Down Expand Up @@ -36,13 +38,24 @@ void testCloseRemote(
[CloseReceived(4123, 'server closed the connection')]);
});

test('send after close received', () async {
test('sendBytes after close received', () async {
final channel = await channelFactory(uri);

channel.sendBytes(Uint8List(10));
expect(await channel.events.toList(),
[CloseReceived(4123, 'server closed the connection')]);
expect(() => channel.sendText('test'),
throwsA(isA<WebSocketConnectionClosed>()));
});

test('sendText after close received', () async {
final channel = await channelFactory(uri);

channel.sendText('Please close');
expect(await channel.events.toList(),
[CloseReceived(4123, 'server closed the connection')]);
expect(() => channel.sendText('test'), throwsStateError);
expect(() => channel.sendText('test'),
throwsA(isA<WebSocketConnectionClosed>()));
});

test('close after close received', () async {
Expand All @@ -51,7 +64,8 @@ void testCloseRemote(
channel.sendText('Please close');
expect(await channel.events.toList(),
[CloseReceived(4123, 'server closed the connection')]);
await expectLater(channel.close, throwsStateError);
await expectLater(
channel.close, throwsA(isA<WebSocketConnectionClosed>()));
});
});
}

0 comments on commit e459e5c

Please sign in to comment.