Skip to content

Conversation

@baileympearson
Copy link
Contributor

@baileympearson baileympearson commented Nov 21, 2025

Description

Summary of Changes

While working on <>, I was reviewing our SDAM error handling for the first time in a while. Our implementation is slightly from the pseudocode, which made cross referencing them challenging. I refactored the SDAM error handling the best I could:

  • All function names now match the spec (i.e., isSDAMUnrecoverableError became isStateChangeError).
  • In a similar vein, I've consolidated all "stale error" related logic into a single function to match the spec's implementation.
  • I've re-ordered the if-statement in our error handling to be in the same order as the spec's pseudocode.

Someday I'd like to finish this alignment. The largest barrier to fully aligning the implementation with the spec's is that in Node, clearing the pool is done by attaching the ResetPool label to an error and calling markServerUnknown. In the spec algorithm:

markServerUnknown();
clearPool();

This is the last real difference between our implementation and the spec's pseudocode but this refactor would be much more invasive than the refactor in this PR, so I've deferred it for now.

Notes for Reviewers

What is the motivation for this change?

Release Highlight

Release notes highlight

Double check the following

  • Lint is passing (npm run check:lint)
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: type(NODE-xxxx)[!]: description
    • Example: feat(NODE-1234)!: rewriting everything in coffeescript
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

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.

2 participants