Skip to content

Commit

Permalink
fix: do not emit error when RelayProxyEndpointsWithoutEvents is used
Browse files Browse the repository at this point in the history
  • Loading branch information
cwaldren-ld committed Mar 5, 2024
1 parent 7a7aa61 commit ae2bb13
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 19 deletions.
28 changes: 25 additions & 3 deletions interfaces/service_endpoints.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,31 @@ package interfaces
// values such as Streaming, or use the helper method
// [github.com/launchdarkly/go-server-sdk/v7/ldcomponents.RelayProxyEndpoints].
//
// Important note: if one or more URI is set to a custom value, then all URIs should be set to custom values.
// Otherwise, the SDK will emit an error-level log to surface this potential misconfiguration, while using default values for
// the unset URIs.
//
// There are some scenarios where it is desirable to set only some of the fields, but this is not recommended for general
// usage. If you're scenario requires it, you can call [WithPartialSpecification] to suppress the
// error message.
//
// See Config.ServiceEndpoints for more details.
type ServiceEndpoints struct {
Streaming string
Polling string
Events string
Streaming string
Polling string
Events string
allowPartialSpecification bool
}

// WithPartialSpecification returns a copy of this ServiceEndpoints that will not trigger an error-level log message
// if only some, but not all the fields are set to custom values. This is an advanced configuration and likely not necessary
// for most use-cases.
func (s ServiceEndpoints) WithPartialSpecification() ServiceEndpoints {
s.allowPartialSpecification = true
return s
}

// PartialSpecificationRequested returns true if this ServiceEndpoints should not be treated as malformed if some, but not all fields are set.
func (s ServiceEndpoints) PartialSpecificationRequested() bool {
return s.allowPartialSpecification
}
10 changes: 6 additions & 4 deletions internal/endpoints/configure_endpoints.go
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,12 @@ func SelectBaseURI(
if anyCustom(serviceEndpoints) {
configuredBaseURI = getCustom(serviceEndpoints, serviceType)
if configuredBaseURI == "" {
loggers.Errorf(
"You have set custom ServiceEndpoints without specifying the %s base URI; connections may not work properly",
serviceType,
)
if !serviceEndpoints.PartialSpecificationRequested() {
loggers.Errorf(
"You have set custom ServiceEndpoints without specifying the %s base URI; connections may not work properly",
serviceType,
)
}
configuredBaseURI = DefaultBaseURI(serviceType)
}
} else {
Expand Down
35 changes: 25 additions & 10 deletions internal/endpoints/configure_endpoints_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func TestSelectCustomURIs(t *testing.T) {
}

func TestLogErrorIfAtLeastOneButNotAllCustomURISpecified(t *testing.T) {
logger := ldlogtest.NewMockLog()

const customURI = "http://custom_uri"

cases := []struct {
Expand All @@ -60,14 +60,29 @@ func TestLogErrorIfAtLeastOneButNotAllCustomURISpecified(t *testing.T) {
{interfaces.ServiceEndpoints{Streaming: customURI, Polling: customURI}, EventsService},
}

// Even if the configuration is considered to be likely malformed, we should still return the proper default URI for
// the service that wasn't configured.
for _, c := range cases {
assert.Equal(t, strings.TrimSuffix(DefaultBaseURI(c.service), "/"), SelectBaseURI(c.endpoints, c.service, logger.Loggers))
}
t.Run("without explicit partial specification", func(t *testing.T) {
logger := ldlogtest.NewMockLog()

// Even if the configuration is considered to be likely malformed, we should still return the proper default URI for
// the service that wasn't configured.
for _, c := range cases {
assert.Equal(t, strings.TrimSuffix(DefaultBaseURI(c.service), "/"), SelectBaseURI(c.endpoints, c.service, logger.Loggers))
}

// For each service that wasn't configured, we should see a log message indicating that.
for _, c := range cases {
logger.AssertMessageMatch(t, true, ldlog.Error, fmt.Sprintf("You have set custom ServiceEndpoints without specifying the %s base URI", c.service))
}
})

t.Run("with partial specification", func(t *testing.T) {
logger := ldlogtest.NewMockLog()

for _, c := range cases {
endpoints := c.endpoints.WithPartialSpecification()
assert.Equal(t, strings.TrimSuffix(DefaultBaseURI(c.service), "/"), SelectBaseURI(endpoints, c.service, logger.Loggers))
}
assert.Empty(t, logger.GetOutput(ldlog.Error))
})

// For each service that wasn't configured, we should see a log message indicating that.
for _, c := range cases {
logger.AssertMessageMatch(t, true, ldlog.Error, fmt.Sprintf("You have set custom ServiceEndpoints without specifying the %s base URI", c.service))
}
}
5 changes: 3 additions & 2 deletions ldcomponents/service_endpoints.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ func RelayProxyEndpoints(relayProxyBaseURI string) interfaces.ServiceEndpoints {
}

// RelayProxyEndpointsWithoutEvents specifies a single base URI for a Relay Proxy instance, telling
// the SDK to use the Relay Proxy for all services except analytics events.
// the SDK to use the Relay Proxy for all services except analytics events. Note that this does not disable events, it
// instead means events (if enabled) will be sent directly to LaunchDarkly.
//
// When using the LaunchDarkly Relay Proxy (https://docs.launchdarkly.com/home/relay-proxy), the SDK
// only needs to know the single base URI of the Relay Proxy, which will provide all of the proxied
Expand All @@ -51,5 +52,5 @@ func RelayProxyEndpointsWithoutEvents(relayProxyBaseURI string) interfaces.Servi
return interfaces.ServiceEndpoints{
Streaming: relayProxyBaseURI,
Polling: relayProxyBaseURI,
}
}.WithPartialSpecification()
}

0 comments on commit ae2bb13

Please sign in to comment.