Skip to content

fix: prevent SSE stream closing before last write completes#888

Open
ehsavoie wants to merge 1 commit into
a2aproject:mainfrom
ehsavoie:falky_test_sse
Open

fix: prevent SSE stream closing before last write completes#888
ehsavoie wants to merge 1 commit into
a2aproject:mainfrom
ehsavoie:falky_test_sse

Conversation

@ehsavoie
Copy link
Copy Markdown
Collaborator

Replace the fragile 150ms sleep workaround in EventConsumer with a
CAS-based state machine in the SSE subscribers that defers response.end()
until the last response.write() handler fires. This fixes the race where
onComplete() could close the HTTP response while a write was still
in-flight, causing clients to see COMPLETED tasks with empty artifacts.

  Replace the fragile 150ms sleep workaround in EventConsumer with a
  CAS-based state machine in the SSE subscribers that defers response.end()
  until the last response.write() handler fires. This fixes the race where
  onComplete() could close the HTTP response while a write was still
  in-flight, causing clients to see COMPLETED tasks with empty artifacts.

Signed-off-by: Emmanuel Hugonnet <ehugonne@redhat.com>
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request replaces a manual sleep workaround in EventConsumer with a state-machine approach in the SSE transport layer to properly handle write completions. It introduces AtomicInteger state tracking and a thread-safe endResponse method across several route files. Feedback indicates that the newly added state constants are duplicated across multiple files and should be moved to a shared location to improve maintainability.

Comment on lines +345 to +347
private static final int IDLE = 0;
private static final int WRITE_PENDING = 1;
private static final int COMPLETE_DEFERRED = 2;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

These constants are duplicated across multiple files. Please move them to a shared location (e.g., a common constants class or a shared module) to ensure consistency and avoid duplication.

References
  1. Constants that are used across multiple modules or components should be moved to a shared location (e.g., a common constants class or a shared module) to avoid duplication and ensure consistency.

Comment on lines +461 to +463
private static final int IDLE = 0;
private static final int WRITE_PENDING = 1;
private static final int COMPLETE_DEFERRED = 2;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

These constants are duplicated across multiple files. Please move them to a shared location (e.g., a common constants class or a shared module) to ensure consistency and avoid duplication.

References
  1. Constants that are used across multiple modules or components should be moved to a shared location (e.g., a common constants class or a shared module) to avoid duplication and ensure consistency.

Comment on lines +811 to +813
private static final int IDLE = 0;
private static final int WRITE_PENDING = 1;
private static final int COMPLETE_DEFERRED = 2;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

These constants are duplicated across multiple files. Please move them to a shared location (e.g., a common constants class or a shared module) to ensure consistency and avoid duplication.

References
  1. Constants that are used across multiple modules or components should be moved to a shared location (e.g., a common constants class or a shared module) to avoid duplication and ensure consistency.

Comment on lines +1001 to +1003
private static final int IDLE = 0;
private static final int WRITE_PENDING = 1;
private static final int COMPLETE_DEFERRED = 2;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

These constants are duplicated across multiple files. Please move them to a shared location (e.g., a common constants class or a shared module) to ensure consistency and avoid duplication.

References
  1. Constants that are used across multiple modules or components should be moved to a shared location (e.g., a common constants class or a shared module) to avoid duplication and ensure consistency.

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.

1 participant