Skip to content

fix(socketio): concurrent acknowledgement timeouts#709

Merged
Totodore merged 8 commits intomainfrom
concurrent-timeouts
Apr 12, 2026
Merged

fix(socketio): concurrent acknowledgement timeouts#709
Totodore merged 8 commits intomainfrom
concurrent-timeouts

Conversation

@Totodore
Copy link
Copy Markdown
Owner

Motivation

When broadcasting with acks, the adapter request timeout is applied on the acknowledgement stream. By default, request_timeout = 5s and ack_timeout = 5s. Because the two timeouts are independently enforced, this leads to the request timeouts not respecting the ack timeout and creates confusing errors.

Solution

  • Add a getter method to get the globally configured timeout.
  • The ack stream timeout is now self.config.request_timeout + timeout.unwrap_or(self.local.ack_timeout()) for redis and mongodb adapters

@Totodore Totodore requested a review from Copilot April 12, 2026 09:55
@Totodore Totodore added C-Bug Something isn't working A-socketioxide Area related to socketioxide A-redis-adapter Area related to redis-adapter A-mongodb-adapter Area related to mongodb-adapter labels Apr 12, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR resolves confusing timeout behavior when broadcasting with acknowledgements across remote adapters by aligning ack-stream timeouts with both the adapter request timeout and the ack timeout.

Changes:

  • Expose the globally configured ack timeout via a new SocketEmitter::ack_timeout() API (and forward it through CoreLocalAdapter).
  • Update Redis and MongoDB adapters to use request_timeout + ack_timeout for the ack stream timeout.
  • Bump crate versions and update changelogs/lockfile accordingly.

Reviewed changes

Copilot reviewed 14 out of 15 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
crates/socketioxide/src/ns.rs Implements the new SocketEmitter::ack_timeout() getter on the server-side emitter.
crates/socketioxide/src/ack.rs Minor rename/clarification of the timeout parameter used for per-socket ack waiting.
crates/socketioxide/CHANGELOG.md Notes the newly exposed ack-timeout getter.
crates/socketioxide/Cargo.toml Updates dependency version for socketioxide-core.
crates/socketioxide-redis/src/lib.rs Adjusts ack-stream timeout to include request timeout + (custom/default) ack timeout.
crates/socketioxide-redis/CHANGELOG.md Adds release note for the ack/request timeout race fix.
crates/socketioxide-redis/Cargo.toml Bumps crate version and core dependency version.
crates/socketioxide-mongodb/tests/fixture.rs Adds a test fixture helper to configure request timeout.
crates/socketioxide-mongodb/src/lib.rs Adjusts ack-stream timeout to include request timeout + (custom/default) ack timeout.
crates/socketioxide-mongodb/CHANGELOG.md Adds release note for the ack/request timeout race fix.
crates/socketioxide-mongodb/Cargo.toml Bumps crate version and core dependency version.
crates/socketioxide-core/src/adapter/mod.rs Adds ack_timeout() to the SocketEmitter trait and forwards via CoreLocalAdapter.
crates/socketioxide-core/CHANGELOG.md Adds a changelog entry for the new ack-timeout exposure.
crates/socketioxide-core/Cargo.toml Bumps the core crate version.
Cargo.lock Updates resolved versions for bumped crates.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread crates/socketioxide-core/CHANGELOG.md Outdated
Comment thread crates/socketioxide-core/Cargo.toml Outdated
Comment thread crates/socketioxide-redis/src/lib.rs Outdated
Comment thread crates/socketioxide-mongodb/src/lib.rs Outdated
Comment thread crates/socketioxide-mongodb/CHANGELOG.md Outdated
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq bot commented Apr 12, 2026

Merging this PR will not alter performance

✅ 87 untouched benchmarks


Comparing concurrent-timeouts (43de457) with main (190b0a3)

Open in CodSpeed

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 20 out of 21 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread crates/parser-msgpack/CHANGELOG.md Outdated
Comment thread crates/parser-common/CHANGELOG.md Outdated
Totodore and others added 2 commits April 12, 2026 13:36
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@Totodore Totodore merged commit e250da1 into main Apr 12, 2026
19 checks passed
@Totodore Totodore deleted the concurrent-timeouts branch April 12, 2026 11:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-mongodb-adapter Area related to mongodb-adapter A-redis-adapter Area related to redis-adapter A-socketioxide Area related to socketioxide C-Bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants