Apply suggestions from code review

Co-authored-by: Greg Wilkins <[email protected]>
jakartaee · Jan 17, 2024 · 162a20a · 162a20a
1 parent d7fbce3
commit 162a20a
Show file tree

Hide file tree

Showing 3 changed files with 43 additions and 35 deletions.
diff --git a/api/src/main/java/jakarta/servlet/ReadListener.java b/api/src/main/java/jakarta/servlet/ReadListener.java
@@ -50,19 +50,22 @@ public interface ReadListener extends EventListener {
     void onAllDataRead() throws IOException;
 
     /**
-     * Invoked when an error occurs reading data. This listener will be invoked if there is a problem with the underlying
-     * connection while data is being read from the stream. We consider data to be being read when the following conditions
-     * are met:
-     *
-     * <ul>
-     * <li>{@link ServletInputStream#isReady()} has been invoked and returned false</li>
-     * <li>{@link ServletInputStream#close()} has not been called</li>
-     * <li>{@link ServletInputStream#read()} (or any other read method) has not returned {@code -1}</li>
-     * </ul>
-     *
-     * If these conditions are not met and the stream is still open then any failure notification will not be delivered
-     * until {@link ServletInputStream#isReady()} is invoked. {@code isReady} must return false in this situation, and then
-     * the failure will be delivered to this method.
+      * Invoked when an error occurs reading data after {@link #setReadListener(ReadListener)} 
+      * has been called.  This method will be invoked if there is a problem while data is being 
+      * read from the stream and either:
+      * <ul>
+      * <li>{@link ServletInputStream#isReady()} has been invoked and returned false;</li>
+      * <li>or {@link ServletInputStream#close()} has been called, and the failure occurred 
+      *       before the response could be completed</li>
+      * </ul>
+      * If these conditions are not met and the stream is still open then any failure
+      * notification will be delivered either:
+      * by an exception thrown from a {@code IO} operation after an invocation of 
+      * {@link ServletInputStream#isReady()} has returned {@code true}; or by a call to this method 
+      * after an invocation of {@link ServletInputStream#isReady()} has returned {@code false};
+      * <p>
+      * This method will not be invoked in any circumstances after 
+      * {@link AsyncListener#onComplete(AsyncEvent)} has been called.
      *
      * @param t the throwable to indicate why the read operation failed
      */

diff --git a/api/src/main/java/jakarta/servlet/ServletInputStream.java b/api/src/main/java/jakarta/servlet/ServletInputStream.java
@@ -178,11 +178,13 @@ public int readLine(byte[] b, int off, int len) throws IOException {
      * If an attempt is made to read from the stream when the stream is in async mode and this method has not returned
      * {@code true} the method will throw an {@link IllegalStateException}.
      * <p>
-     * If an error occurs and {@link ReadListener#onError(Throwable)} is invoked then this method will always return false,
-     * as no further IO operations are allowed after {@code onError} notification.
+    * If an error occurs then either: this method will return {@code true} and 
+    * an {@link IOException} will be thrown from a subsequent call to {@code read(...)};  
+    * or this method will return {@code false} and subsequently {@link ReadListener#onError(Throwable)} 
+    * will be invoked with the error.  Once the error has either been thrown or passed to 
+    * {@link ReadListener#onError(Throwable)}, then this method will always return {@code true} and all 
+    * further {@code read} operations will throw an {@link IOException}.
      * <p>
-     * Note that due to the requirement for {@code read} to never throw in async mode, this method must return false if a
-     * call to {@code read} would result in an exception.
      *
      * @return {@code true} if data can be obtained without blocking, otherwise returns {@code false}.
      * @see ReadListener
@@ -194,11 +196,11 @@ public int readLine(byte[] b, int off, int len) throws IOException {
      * Instructs the <code>ServletInputStream</code> to invoke the provided {@link ReadListener} when it is possible to
      * read.
      * <p>
-     * Note that after this method has been called methods on this stream that are documented to throw {@link IOException}
-     * will no longer throw these exceptions directly, instead any exception that occurs will be reported via
-     * {@link ReadListener#onError(Throwable)}. Please refer to this method for more information. This only applies to
-     * {@code IOException}, other exception types may still be thrown (e.g. methods can throw {@link IllegalStateException}
-     * if {@link #isReady()} has not returned true).
+     * Note that after this method has been called, methods on this stream that are documented 
+     * to throw {@link IOException} might not throw these exceptions directly, 
+     * instead they may be reported via {@link ReadListener#onError(Throwable)} after a call to 
+     * {@link #isReady()} has returned {@code false}. 
+     * Please refer to {@link #isReady()} method for more information. 
      *
      * @param readListener the {@link ReadListener} that should be notified when it's possible to read.
      *

diff --git a/api/src/main/java/jakarta/servlet/ServletOutputStream.java b/api/src/main/java/jakarta/servlet/ServletOutputStream.java
@@ -340,11 +340,12 @@ public void println(double d) throws IOException {
      * If an attempt is made to write to the stream when the stream is in async mode and this method has not returned
      * {@code true} the method will throw an {@link IllegalStateException}.
      * <p>
-     * If an error occurs and {@link WriteListener#onError(Throwable)} is invoked then this method will always return false,
-     * as no further IO operations are allowed after {@code onError} notification.
-     * <p>
-     * Note that due to the requirement for {@code write} to never throw in async mode, this method must return false if a
-     * call to {@code write} would result in an exception.
+    * If an error occurs then either: this method will return {@code true} and 
+    * an {@link IOException} will be thrown from a subsequent call to {@code write(...)};  
+    * or this method will return {@code false} and subsequently {@link WriteListener#onError(Throwable)} 
+    * will be invoked with the error.  Once the error has either been thrown or passed to 
+    * {@link WriteListener#onError(Throwable)}, then this method will always return {@code true} and all 
+    * further {@code write} operations will throw an {@link IOException}.
      *
      * @return {@code true} if data can be written without blocking, otherwise returns {@code false}.
      * @see WriteListener
@@ -356,15 +357,17 @@ public void println(double d) throws IOException {
      * Instructs the <code>ServletOutputStream</code> to invoke the provided {@link WriteListener} when it is possible to
      * write.
      * <p>
-     * Note that after this method has been called methods on this stream that are documented to throw {@link IOException}
-     * will no longer throw these exceptions directly, instead any exception that occurs will be reported via
-     * {@link WriteListener#onError(Throwable)}. Please refer to this method for more information. This only applies to
-     * {@code IOException}, other exception types may still be thrown (e.g. methods can throw {@link IllegalStateException}
-     * if {@link #isReady()} has not returned true).
+     * Note that after this method has been called, methods on this stream that are documented 
+     * to throw {@link IOException} might not throw these exceptions directly, 
+     * instead they may be reported via {@link ReadListener#onError(Throwable)} after a call to 
+     * {@link #isReady()} has returned {@code false}. 
+     * Please refer to {@link #isReady()} method for more information. 
      * <p>
-     * Once this method has been called {@link #flush()} and {@link #close()} become asynchronous operations, they will be
-     * performed in the background and any problems will be reported through the {@link WriteListener#onError(Throwable)}
-     * method.
+     * Once this method has been called {@link #flush()} and {@link #close()} become asynchronous 
+     * operations, possibly performed in the background. Thus any problems may be reported through the 
+     * {@link WriteListener#onError(Throwable)} method, which may be called at any time 
+     * after a {@link #close()} or otherwise only after a call to {@link #isReady()} has returned
+     * {@code false}.
      *
      * @param writeListener the {@link WriteListener} that should be notified when it's possible to write
      *