Skip to content

feat: implement sender and receiver policies based on v1 result codes#514

Merged
b-garbacz merged 11 commits intoimprove-networking-layer-v2from
491-implement-sender-polices-based-on-v1-resultcodes
Mar 6, 2026
Merged

feat: implement sender and receiver policies based on v1 result codes#514
b-garbacz merged 11 commits intoimprove-networking-layer-v2from
491-implement-sender-polices-based-on-v1-resultcodes

Conversation

@b-garbacz
Copy link
Contributor

@b-garbacz b-garbacz commented Mar 3, 2026

PR Description

This PR improves consistency in v1/network error handling and connection policies, while increasing test reliability.

What changed

  • Aligned v1 result codes and overall error handling.
  • Replaced ResultCodePolicy with connectionPolicies and migrated related tests.
  • Introduced custom errors for ConnectionManager.
  • Updated MessageOrchestrator:
    • retry behavior,
    • validator selection,
  • Refactored message broadcast failure messages for clarity.
  • Enhanced account endpoint tests to cover internal error scenarios.
  • Updated sendSingleMessage documentation for clarity and detail.

Outcome

More consistent error/result behavior, cleaner networking code, and more stable test coverage.

@b-garbacz b-garbacz marked this pull request as draft March 3, 2026 13:16
@leonardostsouza leonardostsouza force-pushed the 491-implement-sender-polices-based-on-v1-resultcodes branch from ceddbd5 to 4e95dc9 Compare March 6, 2026 10:28
@b-garbacz b-garbacz requested a review from jusufsuljic March 6, 2026 11:50
@b-garbacz b-garbacz force-pushed the 491-implement-sender-polices-based-on-v1-resultcodes branch from 87ada5b to ab174e5 Compare March 6, 2026 12:13
@b-garbacz b-garbacz marked this pull request as ready for review March 6, 2026 12:13
const protocolError = this.#toProtocolError(error);
const rejected = this.#pendingRequestService.rejectPendingRequest(messageId, protocolError);
if (!rejected) return;
if (shouldEndConnection(protocolError)) connection.end();
Copy link
Contributor

Choose a reason for hiding this comment

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

So, if this line disappeared. Cant you just remove the flag from the Error? In general, that is also not great in terms of design. Exceptions (or things that throw) are done in a pub-sub design. That means that the emitter should work as broadcast and the observer (subscriber) should know how to handle those accordingly. If you pass a behavior flag, you are making it so the emitter know the behavior of the subscriber beforehand.

Copy link
Contributor

Choose a reason for hiding this comment

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

We could maybe check the result codes here and decide whether to close the connection.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, so the plan is to create a new module that defines the behavior based on V1ProtocolError, in a similar way to ResultCodePolicy.js It would be even better to rename that file to ResultCodesPolicies.js.

In that module, we would have:

  • a sender connection handling policy (Success, Rotate, NoRotate)
  • a validator connection handling policy (the ResultCodes that should cause us to close the validator connection).

It will have one big benefit - centralizing all policies into a single file.

@leonardotc @jusufsuljic @leonardostsouza I’m going to merge the current PR as is, and then create a followup issue to refactor it in this direction.

@b-garbacz b-garbacz changed the title feat: implement sender and reciver policies based on v1 result codes feat: implement sender and receiver policies based on v1 result codes Mar 6, 2026
@b-garbacz b-garbacz merged commit 855b54b into improve-networking-layer-v2 Mar 6, 2026
2 of 5 checks passed
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.

4 participants