From dc484ee09e1838dbd8167165cac47e279c834f52 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 18 Feb 2022 12:13:23 -0500 Subject: [PATCH] rpc: set response to nil when not found Otherwise when the query times out we might incorrectly send a value for the reply, when we should send an empty reply. Also document errNotFound and how to handle the result in that case. --- agent/consul/config_endpoint.go | 4 +--- agent/consul/federation_state_endpoint.go | 4 +--- agent/consul/rpc.go | 12 +++++++++++- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/agent/consul/config_endpoint.go b/agent/consul/config_endpoint.go index 419e0afefe..00316c082d 100644 --- a/agent/consul/config_endpoint.go +++ b/agent/consul/config_endpoint.go @@ -205,12 +205,10 @@ func (c *ConfigEntry) Get(args *structs.ConfigEntryQuery, reply *structs.ConfigE return err } - reply.Index = index + reply.Index, reply.Entry = index, entry if entry == nil { return errNotFound } - - reply.Entry = entry return nil }) } diff --git a/agent/consul/federation_state_endpoint.go b/agent/consul/federation_state_endpoint.go index ac02ee469a..ccbbfff13c 100644 --- a/agent/consul/federation_state_endpoint.go +++ b/agent/consul/federation_state_endpoint.go @@ -122,12 +122,10 @@ func (c *FederationState) Get(args *structs.FederationStateQuery, reply *structs return err } - reply.Index = index + reply.Index, reply.State = index, fedState if fedState == nil { return errNotFound } - - reply.State = fedState return nil }) } diff --git a/agent/consul/rpc.go b/agent/consul/rpc.go index 2c15afe5da..d0977e9751 100644 --- a/agent/consul/rpc.go +++ b/agent/consul/rpc.go @@ -945,7 +945,17 @@ type blockingQueryResponseMeta interface { // // The query function is expected to be a closure that has access to responseMeta // so that it can set the Index. The actual result of the query is opaque to blockingQuery. -// If query function returns an error, the error is returned to the caller immediately. +// +// The query function can return errNotFound, which is a sentinel error. Returning +// errNotFound indicates that the query found no results, which allows +// blockingQuery to keep blocking until the query returns a non-nil error. +// The query function must take care to set the actual result of the query to +// nil in these cases, otherwise when blockingQuery times out it may return +// a previous result. errNotFound will never be returned to the caller, it is +// converted to nil before returning. +// +// If query function returns any other error, the error is returned to the caller +// immediately. // // The query function must follow these rules: //