[DRIVERS-2247] Add tests for non-retryable handshake errors Created: 25/Mar/22 Updated: 25/Apr/22 |
|
| Status: | Backlog |
| Project: | Drivers |
| Component/s: | Retryability |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Unknown |
| Reporter: | Daria Pardue | Assignee: | Unassigned |
| Resolution: | Unresolved | Votes: | 0 |
| Labels: | leads-triage | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Issue Links: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Driver Changes: | Needed | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Driver Compliance: |
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Description |
SummaryDRIVERS-746 introduced spec language around retrying handshake errors, and there is still some ongoing discussion regarding whether that specification is complete. Regardless of the specific criteria for retryable errors, it is clear that not all handshake errors should be retryable (depending on the error type as well as the retryable reads/writes client settings); however, there are currently no spec tests for asserting that a driver should not retry particular handshake errors given some set of conditions. This could result in driver implementations that incorrectly retry on every handshake error regardless of error type, code, or client retryability settings. Is this ticket only for tests?Yes |
| Comments |
| Comment by Shane Harvey [ 13/Apr/22 ] |
|
This does not seem urgent but at the same time a new test for a non-retryable error should be fairly straightforward to add. If someone has the time it seems fine to pick up. |
| Comment by Jeremy Mikola [ 02/Apr/22 ] |
|
shane.harvey suggested this might not actually be urgent, as the worst case is having drivers retry handshake errors they otherwise should not (which may be beneficial to users). He also referenced DRIVERS-1842, which pertains to retrying after AuthenticationFailed (context is an inaccessible LDAP server), so I'll mark that as related. |
| Comment by Jeremy Mikola [ 01/Apr/22 ] |
|
daria.pardue: Apologies. I didn't realize we could schedule w/o assigning. Just did so and will alert the leads to talk this over at next week's meeting. |
| Comment by Daria Pardue [ 01/Apr/22 ] |
|
jmikola I would recommend scheduling it or at least pushing it to leads triage; in the Node driver, our first pass at the implementation of DRIVERS-746 ended up retrying everything - we caught it in code review, but it would be good to have actual spec tests that catch it. |
| Comment by Jeremy Mikola [ 01/Apr/22 ] |
|
Moving this to the backlog as a matter of course, but it seems timely given that DRIVERS-746 is currently being implemented. If anyone wants to pick up this spec work or propose that the leads schedule it, feel free to do so. |