fix: prevent SSE stream closing before last write completes#888
fix: prevent SSE stream closing before last write completes#888ehsavoie wants to merge 1 commit into
Conversation
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>
There was a problem hiding this comment.
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.
| private static final int IDLE = 0; | ||
| private static final int WRITE_PENDING = 1; | ||
| private static final int COMPLETE_DEFERRED = 2; |
There was a problem hiding this comment.
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
- 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.
| private static final int IDLE = 0; | ||
| private static final int WRITE_PENDING = 1; | ||
| private static final int COMPLETE_DEFERRED = 2; |
There was a problem hiding this comment.
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
- 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.
| private static final int IDLE = 0; | ||
| private static final int WRITE_PENDING = 1; | ||
| private static final int COMPLETE_DEFERRED = 2; |
There was a problem hiding this comment.
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
- 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.
| private static final int IDLE = 0; | ||
| private static final int WRITE_PENDING = 1; | ||
| private static final int COMPLETE_DEFERRED = 2; |
There was a problem hiding this comment.
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
- 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.
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.