Skip to content

feat: bidirectional StorageController communication with finalize ack#1129

Closed
vringar wants to merge 3 commits intorefactor/event-driven-completionfrom
refactor/bidirectional-extension
Closed

feat: bidirectional StorageController communication with finalize ack#1129
vringar wants to merge 3 commits intorefactor/event-driven-completionfrom
refactor/bidirectional-extension

Conversation

@vringar
Copy link
Contributor

@vringar vringar commented Feb 19, 2026

Summary

  • Add send_to_writer() to socket_interface.py for sending messages on asyncio StreamWriters
  • Add ClientSocket.receive() for receiving messages on an existing connection
  • StorageController sends a finalize_ack back on the StreamWriter after processing a finalize, but only when the client sets want_ack: True
  • Add DataSocket.finalize_visit_id_with_ack() that sends with want_ack and waits for the ack (with timeout fallback)
  • TaskManager uses the ack-aware finalize, confirming data is processed before proceeding
  • Handler catches OSError alongside IncompleteReadError to handle transport poisoning from failed writes

Acks are opt-in (want_ack flag) because writing to a closed connection poisons the shared asyncio transport, preventing reads of remaining buffered messages. Only finalize_visit_id_with_ack sets this flag; fire-and-forget callers (extension, tests) are unaffected.

Stacked on #1128.

@codecov
Copy link

codecov bot commented Feb 20, 2026

Codecov Report

❌ Patch coverage is 13.33333% with 39 lines in your changes missing coverage. Please review.
✅ Project coverage is 39.10%. Comparing base (8a2744c) to head (2bc92b9).

Files with missing lines Patch % Lines
openwpm/socket_interface.py 10.71% 25 Missing ⚠️
openwpm/storage/storage_controller.py 18.75% 13 Missing ⚠️
openwpm/task_manager.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@                         Coverage Diff                          @@
##           refactor/event-driven-completion    #1129      +/-   ##
====================================================================
+ Coverage                             35.25%   39.10%   +3.85%     
====================================================================
  Files                                    39       39              
  Lines                                  3617     3657      +40     
====================================================================
+ Hits                                   1275     1430     +155     
+ Misses                                 2342     2227     -115     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vringar vringar force-pushed the refactor/bidirectional-extension branch from e8a8d80 to 3dd59e3 Compare February 20, 2026 11:06
@vringar vringar force-pushed the refactor/event-driven-completion branch from 31054e1 to e450b63 Compare February 20, 2026 12:17
@vringar vringar force-pushed the refactor/bidirectional-extension branch from 3dd59e3 to 2e0ca20 Compare February 20, 2026 12:17
@vringar vringar changed the base branch from refactor/event-driven-completion to master February 20, 2026 12:21
@vringar vringar changed the base branch from master to refactor/event-driven-completion February 20, 2026 12:24
@vringar vringar force-pushed the refactor/bidirectional-extension branch from 2e0ca20 to 8bda197 Compare February 20, 2026 12:41
@vringar vringar force-pushed the refactor/event-driven-completion branch from e450b63 to 09e5fc7 Compare February 20, 2026 12:41
After processing a finalize message, the StorageController now sends
an ack message back on the StreamWriter to the client that sent it.
This enables bidirectional communication.
DataSocket.finalize_visit_id_with_ack() sends a finalize message
and waits for an acknowledgment from the StorageController. This
replaces the fixed sleep in FinalizeCommand with event-driven
confirmation that data has been processed.
TaskManager.finalize_visit_id now waits for a StorageController ack,
confirming data has been processed before returning. Falls back
gracefully on timeout.
@vringar vringar force-pushed the refactor/event-driven-completion branch from 09e5fc7 to 8a2744c Compare February 20, 2026 12:55
@vringar vringar force-pushed the refactor/bidirectional-extension branch from 8bda197 to 2bc92b9 Compare February 20, 2026 12:55
@vringar vringar closed this Feb 20, 2026
@vringar vringar deleted the refactor/bidirectional-extension branch February 21, 2026 22:08
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