report intermediate error messages during request forwarding (#20643)

* report intermediate error messages during request forwarding

* CL

builtin/logical/pki/crl_util.go

@@ -754,7 +754,7 @@ func (cb *crlBuilder) processRevocationQueue(sc *storageContext) error {
 				}
 
 				if err := sc.Storage.Put(sc.Context, confirmedEntry); err != nil {
-					return fmt.Errorf("error persisting cross-cluster revocation confirmation: %w\nThis may occur when the active node of the primary performance replication cluster is unavailable.", err)
+					return fmt.Errorf("error persisting cross-cluster revocation confirmation: %w", err)
 				}
 			} else {
 				// Since we're the active node of the primary cluster, go ahead

builtin/logical/pki/path_revoke.go

@@ -519,7 +519,7 @@ func (b *backend) maybeRevokeCrossCluster(sc *storageContext, config *crlConfig,
 	}
 
 	if err := sc.Storage.Put(sc.Context, reqEntry); err != nil {
-		return nil, fmt.Errorf("error persisting cross-cluster revocation request: %w\nThis may occur when the active node of the primary performance replication cluster is unavailable.", err)
+		return nil, fmt.Errorf("error persisting cross-cluster revocation request: %w", err)
 	}
 
 	resp := &logical.Response{

changelog/20643.txt

@@ -0,0 +1,3 @@
+```release-note:improvement
+core: report intermediate error messages during request forwarding
+```

sdk/logical/response_util.go

@@ -76,10 +76,21 @@ func RespondErrorCommon(req *Request, resp *Response, err error) (int, error) {
 		var allErrors error
 		var codedErr *ReplicationCodedError
 		errwrap.Walk(err, func(inErr error) {
+			// The Walk function does not just traverse leaves, and execute the
+			// callback function on the entire error first. So, if the error is
+			// of type multierror.Error, we may want to skip storing the entire
+			// error first to avoid adding duplicate errors when walking down
+			// the leaf errors
+			if _, ok := inErr.(*multierror.Error); ok {
+				return
+			}
 			newErr, ok := inErr.(*ReplicationCodedError)
 			if ok {
 				codedErr = newErr
 			} else {
+				// if the error is of type fmt.wrapError which is typically
+				// made by calling fmt.Errorf("... %w", err), allErrors will
+				// contain duplicated error messages
 				allErrors = multierror.Append(allErrors, inErr)
 			}
 		})

vault/request_handling.go

@@ -833,7 +833,30 @@ func (c *Core) doRouting(ctx context.Context, req *logical.Request) (*logical.Re
 	// If we're replicating and we get a read-only error from a backend, need to forward to primary
 	resp, err := c.router.Route(ctx, req)
 	if shouldForward(c, resp, err) {
-		return forward(ctx, c, req)
+		fwdResp, fwdErr := forward(ctx, c, req)
+		if fwdErr != nil && err != logical.ErrReadOnly {
+			// When handling the request locally, we got an error that
+			// contained ErrReadOnly, but had additional information.
+			// Since we've now forwarded this request and got _another_
+			// error, we should tell the user about both errors, so
+			// they know about both.
+			//
+			// When there is no error from forwarding, the request
+			// succeeded and so no additional context is necessary. When
+			// the initial error here was only ErrReadOnly, it's likely
+			// the plugin authors intended to forward this request
+			// remotely anyway.
+			repErr, ok := fwdErr.(*logical.ReplicationCodedError)
+			if ok {
+				fwdErr = &logical.ReplicationCodedError{
+					Msg:  fmt.Sprintf("errors from both primary and secondary; primary error was %s; secondary errors follow: %s", repErr.Error(), err.Error()),
+					Code: repErr.Code,
+				}
+			} else {
+				fwdErr = multierror.Append(fwdErr, err)
+			}
+		}
+		return fwdResp, fwdErr
 	}
 	return resp, err
 }