feat: conform FDv1 streaming and polling to RETRY spec

go.mod

@@ -7,7 +7,7 @@ require (
 	github.com/google/uuid v1.1.1
 	github.com/gregjones/httpcache v0.0.0-20171119193500-2bcd89a1743f
 	github.com/launchdarkly/ccache v1.1.0
-	github.com/launchdarkly/eventsource v1.10.0
+	github.com/launchdarkly/eventsource v1.14.0
 	github.com/launchdarkly/go-jsonstream/v3 v3.1.2
 	github.com/launchdarkly/go-ntlm-proxy-auth v1.0.3
 	github.com/launchdarkly/go-sdk-common/v3 v3.5.1

go.sum

@@ -17,8 +17,8 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
 github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
 github.com/launchdarkly/ccache v1.1.0 h1:voD1M+ZJXR3MREOKtBwgTF9hYHl1jg+vFKS/+VAkR2k=
 github.com/launchdarkly/ccache v1.1.0/go.mod h1:TlxzrlnzvYeXiLHmesMuvoZetu4Z97cV1SsdqqBJi1Q=
-github.com/launchdarkly/eventsource v1.10.0 h1:H9Tp6AfGu/G2qzBJC26iperrvwhzdbiA/gx7qE2nDFI=
-github.com/launchdarkly/eventsource v1.10.0/go.mod h1:J3oa50bPvJesZqNAJtb5btSIo5N6roDWhiAS3IpsKck=
+github.com/launchdarkly/eventsource v1.14.0 h1:6lxVkUwxCAEbBrx8J/N9i25yOajC6NNwq0Voo+fT0Kc=
+github.com/launchdarkly/eventsource v1.14.0/go.mod h1:dU+rZxkPOlGPsyJPpiDqiepAcFwIITDUClY9+A6RrMw=
 github.com/launchdarkly/go-jsonstream/v3 v3.1.2 h1:Od0QEKHesbKnD3GgAt+LkZzhcUiSP+yhrclrhsoox6w=
 github.com/launchdarkly/go-jsonstream/v3 v3.1.2/go.mod h1:ZBjhKq8mhArCtqotGRGnteY6eXpNm1GaOdUSZHh+ZjM=
 github.com/launchdarkly/go-ntlm-proxy-auth v1.0.3 h1:i3V0N+R0Fd2nXfGEVKCBIZ8kyttZ+SRKvBG8cdcphO4=

interfaces/data_source_status_provider.go

@@ -109,8 +109,9 @@ type DataSourceStatus struct {
 	//     state, after previously having been either Initializing or Interrupted.
 	//   - For DataSourceStateInterrupted, it is the time that the data source most recently entered an
 	//     error state, after previously having been Valid.
-	//   - For DataSourceStateOff, it is the time that the data source encountered an unrecoverable error
-	//     or that the SDK was explicitly shut down.
+	//   - For DataSourceStateOff, it is the time that the SDK was explicitly shut down, or (rarely)
+	//     the time that the data source encountered a startup configuration error that prevented any
+	//     connection attempt.
 	StateSince time.Time
 
 	// LastError is information about the last error that the data source encountered, if any.
@@ -141,7 +142,7 @@ const (
 	// initialized.
 	//
 	// If it encounters an error that requires it to retry initialization, the state will remain at
-	// Initializing until it either succeeds and becomes DataSourceStateValid, or permanently fails and
+	// Initializing until it either succeeds and becomes DataSourceStateValid, or permanently stops and
 	// becomes DataSourceStateOff.
 	DataSourceStateInitializing DataSourceState = "INITIALIZING"
 
@@ -163,9 +164,8 @@ const (
 
 	// DataSourceStateOff indicates that the data source has been permanently shut down.
 	//
-	// This could be because it encountered an unrecoverable error (for instance, the LaunchDarkly service
-	// rejected the SDK key; an invalid SDK key will never become valid), or because the SDK client was
-	// explicitly shut down.
+	// This could be because the SDK client was explicitly shut down, or (rarely) because the
+	// data source encountered a startup configuration error that prevented any connection attempt.
 	DataSourceStateOff DataSourceState = "OFF"
 )
 

internal/datasource/helpers.go

@@ -1,6 +1,9 @@
 package datasource
 
 import (
+	"crypto/tls"
+	"crypto/x509"
+	"errors"
 	"fmt"
 	"net/http"
 
@@ -20,22 +23,61 @@ func (e httpStatusError) Error() string {
 	return e.Message
 }
 
-// Tests whether an HTTP error status represents a condition that might resolve on its own if we retry,
-// or at least should not make us permanently stop sending requests.
-func isHTTPErrorRecoverable(statusCode int) bool {
+// FailureClass categorizes a data source failure per RETRY §1.5--§1.7. Under the
+// RETRY spec no failure is permanently terminal: every failure is either "normal"
+// (regular backoff and retry) or "unexpected" (extended backoff via a longer
+// retry profile or wait interval, still retrying indefinitely).
+type FailureClass int
+
+const (
+	// FailureClassNormal indicates a failure that a caller should treat as an
+	// ordinary transient error.
+	FailureClassNormal FailureClass = iota
+	// FailureClassUnexpected indicates a failure that the caller should treat as
+	// signalling a durable, non-transient upstream problem.
+	FailureClassUnexpected
+)
+
+// classifyHTTPFailure returns the failure classification for an HTTP status code
+// received during a data source request, per RETRY §1.6. Called only when the
+// status indicates failure (non-2xx).
+func classifyHTTPFailure(statusCode int) FailureClass {
 	if statusCode >= 400 && statusCode < 500 {
 		switch statusCode {
-		case 400: // bad request
-			return true
-		case 408: // request timeout
-			return true
-		case 429: // too many requests
-			return true
+		case 400, 408, 429:
+			return FailureClassNormal
 		default:
-			return false // all other 4xx errors are unrecoverable
+			return FailureClassUnexpected
 		}
 	}
-	return true
+	return FailureClassNormal
+}
+
+// classifyTransportFailure returns the failure classification for a transport-layer
+// error (i.e., not an HTTP response, but a lower-level network or TLS failure)
+// per RETRY §1.7. TLS/certificate validation failures are treated as unexpected;
+// everything else is treated as normal.
+func classifyTransportFailure(err error) FailureClass {
+	if err == nil {
+		return FailureClassNormal
+	}
+	var certErr *tls.CertificateVerificationError
+	if errors.As(err, &certErr) {
+		return FailureClassUnexpected
+	}
+	var x509UnknownAuthorityErr x509.UnknownAuthorityError
+	if errors.As(err, &x509UnknownAuthorityErr) {
+		return FailureClassUnexpected
+	}
+	var x509HostnameErr x509.HostnameError
+	if errors.As(err, &x509HostnameErr) {
+		return FailureClassUnexpected
+	}
+	var x509InvalidErr x509.CertificateInvalidError
+	if errors.As(err, &x509InvalidErr) {
+		return FailureClassUnexpected
+	}
+	return FailureClassNormal
 }
 
 func httpErrorDescription(statusCode int) string {
@@ -46,27 +88,53 @@ func httpErrorDescription(statusCode int) string {
 	return fmt.Sprintf("HTTP error %d%s", statusCode, message)
 }
 
-// Logs an HTTP error or network error at the appropriate level and determines whether it is recoverable
-// (as defined by isHTTPErrorRecoverable).
-func checkIfErrorIsRecoverableAndLog(
+// classifyAndLogHTTPFailure classifies an HTTP failure per RETRY §1.6, logs it,
+// and returns the classification for the caller to act on. Unexpected
+// failures (401, 403, other 4xx) log at Error since they almost always
+// indicate a real customer-side problem (invalid or expired SDK key,
+// misconfiguration) even though the SDK will keep retrying; normal failures
+// (400, 408, 429, 5xx) log at Warn since they are typically transient.
+func classifyAndLogHTTPFailure(
 	loggers ldlog.Loggers,
 	errorDesc, errorContext string,
 	statusCode int,
-	recoverableMessage string,
-) bool {
-	if statusCode > 0 && !isHTTPErrorRecoverable(statusCode) {
-		loggers.Errorf("Error %s (giving up permanently): %s", errorContext, errorDesc)
-		return false
+	willRetryMessage string,
+) FailureClass {
+	class := classifyHTTPFailure(statusCode)
+	if class == FailureClassUnexpected {
+		loggers.Errorf("Error %s (%s): %s", errorContext, willRetryMessage, errorDesc)
+	} else {
+		loggers.Warnf("Error %s (%s): %s", errorContext, willRetryMessage, errorDesc)
 	}
-	loggers.Warnf("Error %s (%s): %s", errorContext, recoverableMessage, errorDesc)
-	return true
+	return class
+}
+
+// classifyAndLogTransportFailure classifies a transport-layer failure per RETRY
+// §1.7, logs it, and returns the classification. Unexpected failures (TLS
+// or certificate validation errors) log at Error since they almost always
+// indicate a real customer-side problem (misconfigured trust store,
+// expired cert) even though the SDK will keep retrying; other transport
+// failures log at Warn since they are typically transient.
+func classifyAndLogTransportFailure(
+	loggers ldlog.Loggers,
+	err error,
+	errorContext, willRetryMessage string,
+) FailureClass {
+	class := classifyTransportFailure(err)
+	if class == FailureClassUnexpected {
+		loggers.Errorf("Error %s (%s): %s", errorContext, willRetryMessage, err.Error())
+	} else {
+		loggers.Warnf("Error %s (%s): %s", errorContext, willRetryMessage, err.Error())
+	}
+	return class
 }
 
 func checkForHTTPError(statusCode int, url string) error {
 	if statusCode == http.StatusUnauthorized {
 		return httpStatusError{
-			Message: fmt.Sprintf("Invalid SDK key when accessing URL: %s. Verify that your SDK key is correct.", url),
-			Code:    statusCode}
+			Message: fmt.Sprintf("Authentication failed for URL: %s. If this persists, verify that your SDK key is correct.",
+				url),
+			Code: statusCode}
 	}
 
 	if statusCode == http.StatusNotFound {

internal/datasource/helpers_test.go

@@ -1,6 +1,10 @@
 package datasource
 
 import (
+	"crypto/tls"
+	"crypto/x509"
+	"errors"
+	"fmt"
 	"strconv"
 	"testing"
 
@@ -12,15 +16,43 @@ func TestHTTPStatusError(t *testing.T) {
 	assert.Equal(t, "message", error.Error())
 }
 
-func TestIsHTTPErrorRecoverable(t *testing.T) {
+// classifyHTTPFailure per RETRY §1.6: 400, 408, 429 are normal (transient
+// server-side conditions); all other 4xx are unexpected (durable client-side
+// misconfiguration); 5xx and everything else are normal.
+func TestClassifyHTTPFailure(t *testing.T) {
 	for i := 400; i < 500; i++ {
-		assert.Equal(t, i == 400 || i == 408 || i == 429, isHTTPErrorRecoverable(i), strconv.Itoa(i))
+		expected := FailureClassNormal
+		if !(i == 400 || i == 408 || i == 429) {
+			expected = FailureClassUnexpected
+		}
+		assert.Equal(t, expected, classifyHTTPFailure(i), strconv.Itoa(i))
 	}
 	for i := 500; i < 600; i++ {
-		assert.True(t, isHTTPErrorRecoverable(i))
+		assert.Equal(t, FailureClassNormal, classifyHTTPFailure(i), strconv.Itoa(i))
 	}
 }
 
+// classifyTransportFailure per RETRY §1.7: TLS/certificate validation failures
+// are unexpected; other transport-layer errors are normal.
+func TestClassifyTransportFailure(t *testing.T) {
+	assert.Equal(t, FailureClassNormal, classifyTransportFailure(nil))
+	assert.Equal(t, FailureClassNormal, classifyTransportFailure(errors.New("boom")))
+
+	// TLS certificate errors.
+	assert.Equal(t, FailureClassUnexpected,
+		classifyTransportFailure(&tls.CertificateVerificationError{}))
+	assert.Equal(t, FailureClassUnexpected,
+		classifyTransportFailure(x509.UnknownAuthorityError{}))
+	assert.Equal(t, FailureClassUnexpected,
+		classifyTransportFailure(x509.HostnameError{Host: "example.invalid"}))
+	assert.Equal(t, FailureClassUnexpected,
+		classifyTransportFailure(x509.CertificateInvalidError{Reason: x509.Expired}))
+
+	// Wrapped errors are still classified correctly.
+	wrapped := fmt.Errorf("some wrapper: %w", x509.UnknownAuthorityError{})
+	assert.Equal(t, FailureClassUnexpected, classifyTransportFailure(wrapped))
+}
+
 func TestHTTPErrorDescription(t *testing.T) {
 	assert.Equal(t, "HTTP error 400", httpErrorDescription(400))
 	assert.Equal(t, "HTTP error 401 (invalid SDK key)", httpErrorDescription(401))