Uploaded image for project: 'Go Driver'
  1. Go Driver
  2. GODRIVER-3163

Preserve wrappers around context.Canceled & DeadlineExceeded.

    • Type: Icon: Task Task
    • Resolution: Fixed
    • Priority: Icon: Major - P3 Major - P3
    • 1.15.1
    • Affects Version/s: None
    • Component/s: None
    • None
    • Go Drivers
    • Not Needed
    • Hide

      1. What would you like to communicate to the user about this feature?
      2. Would you like the user to see examples of the syntax and/or executable code and its output?
      3. Which versions of the driver/connector does this apply to?

      Show
      1. What would you like to communicate to the user about this feature? 2. Would you like the user to see examples of the syntax and/or executable code and its output? 3. Which versions of the driver/connector does this apply to?

      Context

      REP-3632 alters mongosync’s custom Context implementation so that its Err() returns a wrapper that includes the error’s Cause. The Go driver, though, seems to throw away that wrapper.

      For example here the driver checks if ctx.Err() is Canceled, and if so, it returns a hard-coded Canceled … even though the error check uses errors.Is() in order to (correctly) handle wrapped errors.

      To give a more concrete example: if the context’s Err() returns `context canceled: migration completed successfully`, the driver’s returned error is just `context canceled`.

      Definition of done

      It would be nice to preserve the wrapper.

      While we recognize that it’s a “stretch” for mongosync’s Context.Err() implementation to return a wrapper error instead of, e.g., context.Canceled, the “spirit” of using errors.Is() seems to be that wrappers should be able to substitute for wrapped errors, more or less arbitrarily.

      Potential changeset:

      diff --git a/x/mongo/driver/topology/connection.go b/x/mongo/driver/topology/connection.go
      index 00f44bee..d1a4f726 100644
      --- a/x/mongo/driver/topology/connection.go
      +++ b/x/mongo/driver/topology/connection.go
      @@ -318,7 +318,7 @@ func transformNetworkError(ctx context.Context, originalError error, contextDead
      
              // If there was an error and the context was cancelled, we assume it happened due to the cancellation.
              if errors.Is(ctx.Err(), context.Canceled) {
      -               return context.Canceled
      +               return ctx.Err()
              }
      
              // If there was a timeout error and the context deadline was used, we convert the error into
      @@ -327,7 +327,7 @@ func transformNetworkError(ctx context.Context, originalError error, contextDead
                      return originalError
              }
              if netErr, ok := originalError.(net.Error); ok && netErr.Timeout() {
      -               return context.DeadlineExceeded
      +               return fmt.Errorf("%w: %w", originalError, context.DeadlineExceeded)
              }
      
              return originalError
      

      Pitfalls

      None? Clients who submit stdlib Context structs will be unaffected.

            Assignee:
            matt.dale@mongodb.com Matt Dale
            Reporter:
            felipe.gasper@mongodb.com Felipe Gasper
            Votes:
            0 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated:
              Resolved: