Add stream reconnection repro samples for SubscribeToTaskAsync issues#342
Add stream reconnection repro samples for SubscribeToTaskAsync issues#342SergeyMenshykh wants to merge 1 commit intoa2aproject:mainfrom
Conversation
Add repro methods demonstrating two stream reconnection issues: - SubscribeToTaskAsync hangs indefinitely when reconnecting to an in-progress task (a2aproject#340) - SubscribeToTaskAsync throws UnsupportedOperation when reconnecting to a completed task (a2aproject#341) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds reproduction samples for stream reconnection issues in the AgentClient and updates the AgentServer to simulate slow work and always expose the well-known agent card. Feedback highlights potential runtime exceptions due to null references or empty collections, the use of hardcoded URLs and magic numbers, and an unused variable in the new samples.
| } | ||
|
|
||
| // Reconnect to the stream to resume receiving updates for the same task | ||
| await foreach (var response in agentClient.SubscribeToTaskAsync(new SubscribeToTaskRequest { Id = taskId! })) |
There was a problem hiding this comment.
If SendStreamingMessageAsync yields no items, taskId will be null, and using the null-forgiving operator ! will cause a NullReferenceException at runtime. While this is a repro sample, it's good practice to add a null check for taskId before this line to make the sample more robust. This also applies to Repro_StreamReconnectionFailsForTerminatedTaskAsync.
| // Reconnect to the stream to resume receiving updates for the same task | ||
| await foreach (var response in agentClient.SubscribeToTaskAsync(new SubscribeToTaskRequest { Id = taskId! })) | ||
| { | ||
| Console.WriteLine($" Received task update after reconnection: ID={response.Task!.Id}, Status={response.Task.Status.State}, Artifact={response.Task.Artifacts?[0].Parts?[0].Text}"); |
There was a problem hiding this comment.
This line can throw an IndexOutOfRangeException if response.Task.Artifacts is not null but is an empty list. The null-conditional operator ? only protects against Artifacts being null, not it being empty. Using LINQ's FirstOrDefault() would be safer. This also applies to line 147.
Console.WriteLine($" Received task update after reconnection: ID={response.Task!.Id}, Status={response.Task.Status.State}, Artifact={response.Task.Artifacts?.FirstOrDefault()?.Parts?.FirstOrDefault()?.Text}");|
|
||
| // 3. Create an A2A client to communicate with the echotasks agent using the URL from the agent card | ||
| A2AClient agentClient = new(new Uri(echoAgentCard.SupportedInterfaces[0].Url)); | ||
| A2AClient agentClient = new(new Uri("http://localhost:5101/echotasks")); |
There was a problem hiding this comment.
The agent client URL is hardcoded. It's better to use the URL from the resolved echoAgentCard as it was before. This makes the sample more robust to changes in the agent server's configuration (e.g., port or path).
A2AClient agentClient = new(new Uri(echoAgentCard.SupportedInterfaces[0].Url));|
|
||
| // Simulate a delay before reconnecting. During this time, the task may reach a terminal | ||
| // state (Completed/Failed/Canceled) while the client is unaware due to the disconnection. | ||
| await Task.Delay(5000); |
There was a problem hiding this comment.
The delay duration 5000 is a magic number. It would be better to define it as a named constant to improve readability and maintainability, for example private const int ReconnectDelayMs = 5000;.
|
|
||
| // 5. Demo a long-running task | ||
| await DemoLongRunningTaskAsync(agentClient); | ||
| AgentTask finalTaskState = await agentClient.GetTaskAsync(new GetTaskRequest { Id = taskId! }); |
There was a problem hiding this comment.
The variable finalTaskState is assigned but its value is never used. It should be used (e.g., for logging the final state) to make the sample more illustrative.
AgentTask finalTaskState = await agentClient.GetTaskAsync(new GetTaskRequest { Id = taskId! });
Console.WriteLine($"Retrieved final task state: ID={finalTaskState.Id}, Status={finalTaskState.Status.State}");| await updater.SubmitAsync(cancellationToken); | ||
|
|
||
| await updater.StartWorkAsync(cancellationToken: cancellationToken); | ||
| await Task.Delay(3000, cancellationToken); // simulate slow work |
There was a problem hiding this comment.
The delay 3000 is a magic number. Consider defining it as a named constant to improve readability, e.g., private const int WorkSimulationDelayMs = 3000;.
1 participant
This PR adds repro samples demonstrating two stream reconnection issues with SubscribeToTaskAsync:
The repro methods are added to the TaskBasedCommunicationSample in the AgentClient sample project.