Skip to content

[Feat]: Revisit exception handling in DefaultRequestHandler.onMessageSend() #449

Closed

Description

@kabir

Is your feature request related to a problem? Please describe.

No response

Describe the solution you'd like

In DefaultRequestHandler

} catch (InterruptedException e) {
Thread.currentThread().interrupt();
String msg = String.format("Error waiting for task %s completion", taskId);
LOGGER.warn(msg, e);
throw new InternalError(msg);
} catch (java.util.concurrent.ExecutionException e) {
String msg = String.format("Error during task %s execution", taskId);
LOGGER.warn(msg, e.getCause());
throw new InternalError(msg);
} catch (java.util.concurrent.TimeoutException e) {
String msg = String.format("Timeout waiting for consumption to complete for task %s", taskId);
LOGGER.warn(msg, taskId);
throw new InternalError(msg);
we currently throw InternalError if InterruptedException, TimeoutException or ExecutionException happens.

We were originally just logging the errors, and passed review on main. But then on the 0.3.x backport PR, Gemini had a comment, which led us to adding these errors. However, this doesn't feel very correct.

We need to investigate these cases:

  • TimeoutException - this probably just means that the client timed out, and that the processing of the Task should happen in the background
  • ExecutionException - Possibly, this will be tracked by the handling in DefaultRequestHandler.registerAndExecuteAgentAsync()?
  • InterruptedException - I think this means the sever is shutting down. What does this mean for the Task?

Another thing to note is that with the current aggressive throwing of InternalError in all three cases, we won't invoke PushNotification callbacks

Describe alternatives you've considered

No response

Additional context

No response

Code of Conduct

  • I agree to follow this project's Code of Conduct

Activity

  1. added theissue type on Nov 5, 2025
  2. added this to the 1.0.0 milestone on Dec 11, 2025
  3. kabir commented on Feb 12, 2026

    @kabir
    CollaboratorAuthor

    We have rewritten a lot in this area (in #611 and similar PRs). So I think this is out of date

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    wontfixThis will not be worked on

    Type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions