From d7fbce3221c64fab8489119ad4132d56798025a9 Mon Sep 17 00:00:00 2001 From: Stuart Douglas Date: Wed, 3 Nov 2021 11:00:45 +1100 Subject: [PATCH 1/2] Clarify under what circumstances onError is called Fixes #433 Signed-off-by: Stuart Douglas --- .../java/jakarta/servlet/ReadListener.java | 14 +++++++++++- .../jakarta/servlet/ServletInputStream.java | 18 ++++++++++++++- .../jakarta/servlet/ServletOutputStream.java | 22 +++++++++++++++++-- .../java/jakarta/servlet/WriteListener.java | 14 +++++++++++- spec/src/main/asciidoc/servlet-spec-body.adoc | 3 +++ 5 files changed, 66 insertions(+), 5 deletions(-) diff --git a/api/src/main/java/jakarta/servlet/ReadListener.java b/api/src/main/java/jakarta/servlet/ReadListener.java index 745c19f95..ed91542c9 100644 --- a/api/src/main/java/jakarta/servlet/ReadListener.java +++ b/api/src/main/java/jakarta/servlet/ReadListener.java @@ -50,7 +50,19 @@ public interface ReadListener extends EventListener { void onAllDataRead() throws IOException; /** - * Invoked when an error occurs processing the request. + * 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: + * + * + * + * 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. * * @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 index 02f7eecdf..1e3db363e 100644 --- a/api/src/main/java/jakarta/servlet/ServletInputStream.java +++ b/api/src/main/java/jakarta/servlet/ServletInputStream.java @@ -175,6 +175,15 @@ public int readLine(byte[] b, int off, int len) throws IOException { * available. Other than the initial call, {@link ReadListener#onDataAvailable()} will only be called if and only if * this method is called and returns false. * + * 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}. + *

+ * 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. + *

+ * 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 * @since Servlet 3.1 @@ -182,7 +191,14 @@ public int readLine(byte[] b, int off, int len) throws IOException { public abstract boolean isReady(); /** - * Instructs the ServletInputStream to invoke the provided {@link ReadListener} when it is possible to read + * Instructs the ServletInputStream to invoke the provided {@link ReadListener} when it is possible to + * read. + *

+ * 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). * * @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 index cec5f001a..96c70aed3 100644 --- a/api/src/main/java/jakarta/servlet/ServletOutputStream.java +++ b/api/src/main/java/jakarta/servlet/ServletOutputStream.java @@ -336,6 +336,15 @@ public void println(double d) throws IOException { * {@link WriteListener#onWritePossible()} once a write operation becomes possible without blocking. Other than the * initial call, {@link WriteListener#onWritePossible()} will only be called if and only if this method is called and * returns false. + *

+ * 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}. + *

+ * 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. + *

+ * 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. * * @return {@code true} if data can be written without blocking, otherwise returns {@code false}. * @see WriteListener @@ -345,8 +354,17 @@ public void println(double d) throws IOException { /** * Instructs the ServletOutputStream to invoke the provided {@link WriteListener} when it is possible to - * write - * + * write. + *

+ * 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). + *

+ * 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. * * @param writeListener the {@link WriteListener} that should be notified when it's possible to write * diff --git a/api/src/main/java/jakarta/servlet/WriteListener.java b/api/src/main/java/jakarta/servlet/WriteListener.java index 1b2cd33a8..91d768a2e 100644 --- a/api/src/main/java/jakarta/servlet/WriteListener.java +++ b/api/src/main/java/jakarta/servlet/WriteListener.java @@ -40,7 +40,19 @@ public interface WriteListener extends EventListener { void onWritePossible() throws IOException; /** - * Invoked when an error occurs writing data using the non-blocking APIs. + * Invoked when an error occurs writing data using the non-blocking APIs. This listener will be invoked if there is a + * problem with the underlying connection while data is being written to the stream. We consider data to be being + * written when any of the following conditions are met: + * + *

+ * + * If these conditions are not met and the stream is still open then any failure notification will not be delivered + * until {@link ServletOutputStream#isReady()} is invoked. {@code isReady} must return false in this situation, and then + * the failure will be delivered to the {@link #onError(Throwable)} method. * * @param t the throwable to indicate why the write operation failed */ diff --git a/spec/src/main/asciidoc/servlet-spec-body.adoc b/spec/src/main/asciidoc/servlet-spec-body.adoc index ebe2ce06c..b1b3c2733 100644 --- a/spec/src/main/asciidoc/servlet-spec-body.adoc +++ b/spec/src/main/asciidoc/servlet-spec-body.adoc @@ -8586,6 +8586,9 @@ Jakarta Servlet {spec-version} specification developed under the Jakarta EE Work === Changes Since Jakarta Servlet 6.0 +link:https://github.com/eclipse-ee4j/servlet-api/issues/433[Issue 433]:: +Clarify how IO errors are handled by async streams. + link:https://github.com/eclipse-ee4j/servlet-api/issues/59[Issue 59]:: A new attribute, `jakarta.servlet.error.query_string`, has been added to the attributes that must be set on the request as part of an error dispatch. From 4936159a361af5f87b603b0cb8d58f2dd2e22ac4 Mon Sep 17 00:00:00 2001 From: Stuart Douglas Date: Wed, 17 Jan 2024 13:46:49 +1100 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Greg Wilkins --- .../java/jakarta/servlet/ReadListener.java | 23 ++++++++-------- .../jakarta/servlet/ServletInputStream.java | 18 ++++++------- .../jakarta/servlet/ServletOutputStream.java | 26 +++++++++---------- .../java/jakarta/servlet/WriteListener.java | 16 +++++++----- 4 files changed, 43 insertions(+), 40 deletions(-) diff --git a/api/src/main/java/jakarta/servlet/ReadListener.java b/api/src/main/java/jakarta/servlet/ReadListener.java index ed91542c9..54344dd61 100644 --- a/api/src/main/java/jakarta/servlet/ReadListener.java +++ b/api/src/main/java/jakarta/servlet/ReadListener.java @@ -50,19 +50,20 @@ 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: - * + * 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: *
    - *
  • {@link ServletInputStream#isReady()} has been invoked and returned false
    • - *
  • {@link ServletInputStream#close()} has not been called
    • - *
  • {@link ServletInputStream#read()} (or any other read method) has not returned {@code -1}
    • + *
  • {@link ServletInputStream#isReady()} has been invoked and returned false;
    • + *
  • or {@link ServletInputStream#close()} has been called, and the failure occurred before the response could be + * completed
    • *
- * - * 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. + * 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}; + *

+ * 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 index 1e3db363e..e0df6a8a9 100644 --- a/api/src/main/java/jakarta/servlet/ServletInputStream.java +++ b/api/src/main/java/jakarta/servlet/ServletInputStream.java @@ -178,11 +178,12 @@ 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}. *

- * 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}. *

- * 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 +195,10 @@ public int readLine(byte[] b, int off, int len) throws IOException { * Instructs the ServletInputStream to invoke the provided {@link ReadListener} when it is possible to * read. *

- * 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 index 96c70aed3..60a010e9d 100644 --- a/api/src/main/java/jakarta/servlet/ServletOutputStream.java +++ b/api/src/main/java/jakarta/servlet/ServletOutputStream.java @@ -340,11 +340,11 @@ 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}. *

- * 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. - *

- * 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 +356,15 @@ public void println(double d) throws IOException { * Instructs the ServletOutputStream to invoke the provided {@link WriteListener} when it is possible to * write. *

- * 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. *

- * 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 * diff --git a/api/src/main/java/jakarta/servlet/WriteListener.java b/api/src/main/java/jakarta/servlet/WriteListener.java index 91d768a2e..896d8954a 100644 --- a/api/src/main/java/jakarta/servlet/WriteListener.java +++ b/api/src/main/java/jakarta/servlet/WriteListener.java @@ -40,19 +40,21 @@ public interface WriteListener extends EventListener { void onWritePossible() throws IOException; /** - * Invoked when an error occurs writing data using the non-blocking APIs. This listener will be invoked if there is a - * problem with the underlying connection while data is being written to the stream. We consider data to be being - * written when any of the following conditions are met: - * + * Invoked when an error occurs writing data after {@link ServletOutputStream#setWriteListener(WriteListener)} has been + * called. This method will be invoked if there is a problem while data is being written to the stream and either: *

    *
  • {@link ServletOutputStream#isReady()} has been invoked and returned false
    • *
  • {@link ServletOutputStream#close()} has been called, and the failure occurred before the response could be fully * written to the client
    • *
* - * If these conditions are not met and the stream is still open then any failure notification will not be delivered - * until {@link ServletOutputStream#isReady()} is invoked. {@code isReady} must return false in this situation, and then - * the failure will be delivered to the {@link #onError(Throwable)} method. + * 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 ServletOutputStream#isReady()} has + * returned {@code true}; or by a call to this method after an invocation of {@link ServletOutputStream#isReady()} has + * returned {@code false}; + *

+ * 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 write operation failed */