Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(NODE-4475): make interrupted message more specific #3437

Merged
merged 3 commits into from Oct 13, 2022
Merged

Conversation

durran
Copy link
Member

@durran durran commented Oct 6, 2022

Description

Fixes change stream retryability on 4.4 servers and lower when performing a failover.

What is changing?

This is the "quick and dirty" fix that just avoids pushing a null to the readable cursor stream before calling destroy on the stream when the error contains the message "interrupted at shutdown".

Existing spec tests cover the more specific error - see: https://github.com/mongodb/node-mongodb-native/blob/main/test/spec/retryable-writes/legacy/insertOne-serverErrors.yml#L468

Note: see #3438 for what a start of a refactor away from checking for this error message and creating separate streams for the change stream cursor and the others would look like. However it seems the "operation was interrupted" message affects change streams as well so I'll leave it as draft for now.

Is there new documentation needed for these changes?

None

What is the motivation for this change?

NODE-4475

Double check the following

  • Ran npm run check:lint script
  • Self-review completed using the steps outlined here
  • PR title follows the correct format: <type>(NODE-xxxx)<!>: <description>
  • Changes are covered by tests
  • New TODOs have a related JIRA ticket

@durran durran marked this pull request as ready for review October 6, 2022 05:46
Copy link
Contributor

@baileympearson baileympearson left a comment

Choose a reason for hiding this comment

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

From tech chat - let's try to add tests that

  • confirm a change stream resumes if it encounters a resumable error with interrupted in the error message
  • confirm that a change stream does not resume if it encounters an error with operation was interrupted as the message

Also - @durran want to file a follow up ticket to investigate removing the two special cases here?

@durran
Copy link
Member Author

durran commented Oct 12, 2022

From tech chat - let's try to add tests that

* confirm a change stream resumes if it encounters a resumable error with `interrupted` in the error message

* confirm that a change stream does not resume if it encounters an error with `operation was interrupted` as the message

Also - @durran want to file a follow up ticket to investigate removing the two special cases here?

https://jira.mongodb.org/browse/NODE-4717 is created.

@durran durran added the Primary Review In Review with primary reviewer, not yet ready for team's eyes label Oct 13, 2022
@baileympearson baileympearson added Team Review Needs review from team and removed Primary Review In Review with primary reviewer, not yet ready for team's eyes labels Oct 13, 2022
@dariakp dariakp merged commit 5f37cb6 into main Oct 13, 2022
@dariakp dariakp deleted the NODE-4475 branch October 13, 2022 15:26
ZLY201 pushed a commit to ZLY201/node-mongodb-native that referenced this pull request Nov 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Team Review Needs review from team
Projects
None yet
3 participants