[ VM / dart:io ] Adding to a closed IOSink now throws a StateError

Based off of these original changes:
https://codereview.chromium.org/2857393003

This is a breaking change and will require an annoucement stating such
before landing.

Fixes #29554.

Change-Id: Ibb56fd49648edc6b9fd567240a3bebb05a14234d
Reviewed-on: https://dart-review.googlesource.com/c/90120
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>

CHANGELOG.md

@@ -43,6 +43,9 @@
 #### `dart:io`
 
 *   Added ability to get and set low level socket options.
+* **Breaking Change:** Adding to a closed `IOSink` now throws a `StateError`.
+
+[29554]: https://github.com/dart-lang/sdk/issues/29554
 
 ### Dart VM
 

sdk/lib/_http/http_impl.dart

@@ -531,24 +531,16 @@ class _StreamSinkImpl<T> implements StreamSink<T> {
 
   _StreamSinkImpl(this._target);
 
-  void _reportClosedSink() {
-    stderr.writeln("StreamSink is closed and adding to it is an error.");
-    stderr.writeln("  See http://dartbug.com/29554.");
-    stderr.writeln(StackTrace.current);
-  }
-
   void add(T data) {
     if (_isClosed) {
-      _reportClosedSink();
-      return;
+      throw StateError("StreamSink is closed");
     }
     _controller.add(data);
   }
 
   void addError(error, [StackTrace stackTrace]) {
     if (_isClosed) {
-      _reportClosedSink();
-      return;
+      throw StateError("StreamSink is closed");
     }
     _controller.addError(error, stackTrace);
   }

sdk/lib/async/stream_transformers.dart

@@ -220,17 +220,9 @@ class _HandlerEventSink<S, T> implements EventSink<S> {
 
   bool get _isClosed => _sink == null;
 
-  _reportClosedSink() {
-    // TODO(29554): throw a StateError, and don't just report the problem.
-    Zone.root
-      ..print("Sink is closed and adding to it is an error.")
-      ..print("  See http://dartbug.com/29554.")
-      ..print(StackTrace.current.toString());
-  }
-
   void add(S data) {
     if (_isClosed) {
-      _reportClosedSink();
+      throw StateError("Sink is closed");
     }
     if (_handleData != null) {
       _handleData(data, _sink);
@@ -241,7 +233,7 @@ class _HandlerEventSink<S, T> implements EventSink<S> {
 
   void addError(Object error, [StackTrace stackTrace]) {
     if (_isClosed) {
-      _reportClosedSink();
+      throw StateError("Sink is closed");
     }
     if (_handleError != null) {
       _handleError(error, stackTrace, _sink);

sdk/lib/io/io_sink.dart

@@ -149,32 +149,16 @@ class _StreamSinkImpl<T> implements StreamSink<T> {
 
   _StreamSinkImpl(this._target);
 
-  void _reportClosedSink() {
-    // TODO(29554): this is very brittle and depends on the layout of the
-    // stderr class.
-    if (this == stderr._sink) {
-      // We can't report on stderr anymore (as we would otherwise
-      // have an infinite recursion.
-      throw new StateError("Stderr is closed.");
-    }
-    // TODO(29554): throw a StateError, and don't just report the problem.
-    stderr.writeln("StreamSink is closed and adding to it is an error.");
-    stderr.writeln("  See http://dartbug.com/29554.");
-    stderr.writeln(StackTrace.current);
-  }
-
   void add(T data) {
     if (_isClosed) {
-      _reportClosedSink();
-      return;
+      throw StateError("StreamSink is closed");
     }
     _controller.add(data);
   }
 
   void addError(error, [StackTrace stackTrace]) {
     if (_isClosed) {
-      _reportClosedSink();
-      return;
+      throw StateError("StreamSink is closed");
     }
     _controller.addError(error, stackTrace);
   }

tests/co19_2/co19_2-kernel.status

@@ -1367,7 +1367,6 @@ LibTest/io/HttpClientBasicCredentials/HttpClientBasicCredentials_A01_t01: Runtim
 LibTest/io/HttpClientDigestCredentials/HttpClientDigestCredentials_A01_t01: RuntimeError
 LibTest/io/HttpClientRequest/addStream_A02_t02: RuntimeError
 LibTest/io/HttpClientRequest/add_A03_t01: RuntimeError
-LibTest/io/HttpClientRequest/close_A02_t01: RuntimeError
 LibTest/io/HttpClientRequest/done_A02_t01: RuntimeError, Timeout, Pass
 LibTest/io/HttpClientResponse/certificate_A01_t01: RuntimeError
 LibTest/io/HttpClientResponse/isRedirect_A01_t02: RuntimeError

tests/standalone_2/io/client_socket_exception_test.dart

@@ -47,7 +47,7 @@ void clientSocketExceptionTest() {
       } catch (ex) {
         wrongExceptionCaught = true;
       }
-      Expect.isFalse(exceptionCaught);
+      Expect.isTrue(exceptionCaught);
       Expect.isFalse(wrongExceptionCaught);
 
       // From here exceptions are expected.

tests/standalone_2/io/file_test.dart

@@ -988,7 +988,9 @@ class FileTest {
     file.createSync();
     var output = file.openWrite();
     output.close();
-    output.add(buffer); // Ignored.
+    Expect.throws(() {
+      output.add(buffer);
+    });
     output.done.then((_) {
       file.deleteSync();
       asyncTestDone("testCloseExceptionStream");

tests/standalone_2/io/http_10_test.dart

@@ -29,7 +29,9 @@ void testHttp10NoKeepAlive() {
       response.write("Z");
       response.write("Z");
       response.close();
-      response.write("x");
+      Expect.throws(() {
+        response.write("x");
+      }, (e) => e is StateError);
     }, onError: (e, trace) {
       String msg = "Unexpected error $e";
       if (trace != null) msg += "\nStackTrace: $trace";

tests/standalone_2/io/http_content_length_test.dart

@@ -37,7 +37,9 @@ void testNoBody(int totalConnections, bool explicitContentLength) {
       // After an explicit close, write becomes a state error
       // because we have said we will not add more.
       response.close();
-      response.write("x");
+      Expect.throws(() {
+        response.write("x");
+      }, (e) => e is StateError);
     }, onError: (e, trace) {
       String msg = "Unexpected server error $e";
       if (trace != null) msg += "\nStackTrace: $trace";
@@ -89,7 +91,9 @@ void testBody(int totalConnections, bool useHeader) {
           }
         });
         response.close();
-        response.write("x");
+        Expect.throws(() {
+          response.write("x");
+        }, (e) => e is StateError);
       });
     }, onError: (e, trace) {
       String msg = "Unexpected error $e";
@@ -149,7 +153,9 @@ void testBodyChunked(int totalConnections, bool useHeader) {
         response.write("x");
         response.write("x");
         response.close();
-        response.write("x");
+        Expect.throws(() {
+          response.write("x");
+        }, (e) => e is StateError);
       });
     }, onError: (e, trace) {
       String msg = "Unexpected error $e";

tests/standalone_2/io/socket_upgrade_to_secure_test.dart

@@ -164,7 +164,9 @@ void test(bool hostnameInConnect, bool handshakeBeforeSecure,
               SecureSocket.secure(socket, host: HOST, context: clientContext);
         }
         return future.then<SecureSocket>((SecureSocket secureSocket) {
-          socket.add([0]);
+          Expect.throws(() {
+            socket.add([0]);
+          });
           return secureSocket;
         });
       });
@@ -179,7 +181,9 @@ void test(bool hostnameInConnect, bool handshakeBeforeSecure,
                 SecureSocket.secure(socket, host: HOST, context: clientContext);
           }
           return future.then((secureSocket) {
-            socket.add([0]);
+            Expect.throws(() {
+              socket.add([0]);
+            });
             return secureSocket;
           });
         });
@@ -191,7 +195,9 @@ void test(bool hostnameInConnect, bool handshakeBeforeSecure,
     server.listen((client) {
       if (!handshakeBeforeSecure) {
         SecureSocket.secureServer(client, serverContext).then((secureClient) {
-          client.add([0]);
+          Expect.throws(() {
+            client.add([0]);
+          });
           runServer(secureClient).then((_) => server.close());
         });
       } else {
@@ -199,7 +205,9 @@ void test(bool hostnameInConnect, bool handshakeBeforeSecure,
           SecureSocket
               .secureServer(client, serverContext, bufferedData: carryOverData)
               .then((secureClient) {
-            client.add([0]);
+            Expect.throws(() {
+              client.add([0]);
+            });
             runServer(secureClient).then((_) => server.close());
           });
         });