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

GODRIVER-3163 Preserve original cancellation/timeout errors. #1625

Merged

Conversation

FGasper
Copy link
Contributor

@FGasper FGasper commented Apr 29, 2024

GODRIVER-3163

Summary

Previously transformNetworkError() would replace original errors with either context.Canceled or context.DeadlineExceeded. This changes that so that the original error information is preserved.

Background & Motivation

Use of errors.Is() implies a desire to honor wrapped errors. Replacing a wrapped error with context.Canceled subverts that.

This subversion adversely affects Mongosync, which wraps context.Canceled with detail about the cancellation’s cause. (See REP-3632 for details about this.)

The same pattern applies to discarding the driver’s originalError in favor of DeadlineExceeded. We can safely preserve the original net.Error’s context by wrapping DeadlineExceeded.

@FGasper FGasper force-pushed the GODRIVER-3163-preserve-canceled-wrapper branch from bcf62ef to 7a3d47e Compare April 29, 2024 14:27
@mongodb-drivers-pr-bot mongodb-drivers-pr-bot bot added the priority-3-low Low Priority PR for Review label Apr 29, 2024
Copy link

API Change Report

No changes found!

@FGasper FGasper force-pushed the GODRIVER-3163-preserve-canceled-wrapper branch from 7a3d47e to e7bdf99 Compare April 29, 2024 14:33
@matthewdale matthewdale changed the title Preserve original cancellation/timeout errors. GODRIVER-3163 Preserve original cancellation/timeout errors. Apr 29, 2024
Copy link
Collaborator

@matthewdale matthewdale left a comment

Choose a reason for hiding this comment

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

Looks good! 👍

Note that there are no tests that assert this behavior, so it's possible there could be an undetected regression in this behavior. Since it's not a behavior we officially advertise, I think it's fine to wait and see if that becomes a problem.

@matthewdale matthewdale merged commit 091ec6c into mongodb:v1 May 1, 2024
29 of 33 checks passed
@FGasper FGasper deleted the GODRIVER-3163-preserve-canceled-wrapper branch May 1, 2024 23:55
@blink1073
Copy link
Member

@matthewdale should this be ported to master?

@matthewdale
Copy link
Collaborator

@blink1073 Yes, it should. Thanks for checking!

blink1073 pushed a commit to blink1073/mongo-go-driver that referenced this pull request May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority-3-low Low Priority PR for Review
Projects
None yet
3 participants