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

Fix of exception handler trying to write response to a disconnected c… #109

Merged

Conversation

ivakoleva
Copy link
Contributor

…lient

Pull Request type

  • Bugfix
  • Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • WHOSUSING.md
  • Other (please describe):

NOTE: Please remember to run ./gradlew spotlessApply to fix any format violations.

Changes in this PR

On Conductor server side, an exception gets raised Failure in @ExceptionHandler com.netflix.conductor.rest.controllers.ApplicationExceptionMapper#handleAll(HttpServletRequest, Throwable). Cause is - attempting to write while the client aborted the connection org.apache.catalina.connector.ClientAbortException: java.io.IOException: Broken pipe

Added a ClientAbortException handler.

Testing done: Local run. Not trivial to emulate the issue in a test. Such a test may rely on timing out the client before server responded.

Alternatives considered

…lient

Why:
On Conductor server side, an exception gets raised
`Failure in @ExceptionHandler com.netflix.conductor.rest.controllers.ApplicationExceptionMapper#handleAll(HttpServletRequest, Throwable)`.
Cause is attempting to write while the client aborted the connection
`org.apache.catalina.connector.ClientAbortException: java.io.IOException: Broken pipe`

What:
Added a ClientAbortException handler.

Testing done: Local run. Not trivial to emulate the issue in a test.
Such a test may rely on timing out the client before server responded.

Signed-off-by: Iva Avramova <ikoleva@vmware.com>
@@ -53,7 +53,16 @@ public class ApplicationExceptionMapper {
EXCEPTION_STATUS_MAP.put(NoResourceFoundException.class, HttpStatus.NOT_FOUND);
}

@ExceptionHandler(ClientAbortException.class)
Copy link
Contributor

Choose a reason for hiding this comment

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

Please check the build failure, Probably a testcase is failing

@v1r3n v1r3n merged commit 7e7b88b into conductor-oss:main Apr 3, 2024
1 of 2 checks passed
@manan164
Copy link
Contributor

manan164 commented Apr 3, 2024

Hi @ivakoleva , Do we need this change? Since the exception is clear on the client side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants