From 256f074604c39284f78b412b65a48e481445a514 Mon Sep 17 00:00:00 2001 From: "amin.paydar" Date: Tue, 5 May 2026 16:08:31 +0330 Subject: [PATCH 1/2] feat(fcm): add retry config client option support Allow callers to pass retry policy through option.ClientOption so Messaging can honor custom retry behavior from NewApp options. When no retry option is supplied the default retry config is still applied. --- internal/http_client.go | 67 ++++++++++++++++++++++++++++++------ internal/http_client_test.go | 32 +++++++++++++++++ messaging/messaging.go | 16 +++++---- messaging/messaging_test.go | 43 +++++++++++++++++++++++ messaging/topic_mgt.go | 4 +-- retry_options.go | 31 +++++++++++++++++ 6 files changed, 174 insertions(+), 19 deletions(-) create mode 100644 retry_options.go diff --git a/internal/http_client.go b/internal/http_client.go index 9d1257bc..26ceff10 100644 --- a/internal/http_client.go +++ b/internal/http_client.go @@ -29,6 +29,7 @@ import ( "time" "google.golang.org/api/option" + "google.golang.org/api/option/internaloption" "google.golang.org/api/transport" ) @@ -65,7 +66,13 @@ func NewHTTPClient(ctx context.Context, opts ...option.ClientOption) (*HTTPClien return nil, "", err } - return WithDefaultRetryConfig(hc), endpoint, nil + client := &HTTPClient{Client: hc} + if retryConfig, ok := retryConfigFromOptions(opts...); ok { + client.RetryConfig = retryConfig + } else { + client.RetryConfig = defaultRetryConfig() + } + return client, endpoint, nil } // WithDefaultRetryConfig creates a new HTTPClient using the provided client and the default @@ -75,17 +82,20 @@ func NewHTTPClient(ctx context.Context, opts ...option.ClientOption) (*HTTPClien // ServiceUnavailable (503) error. Repeatedly failing requests are retried up to 4 times // with exponential backoff. Retry delay is never longer than 2 minutes. func WithDefaultRetryConfig(hc *http.Client) *HTTPClient { - twoMinutes := time.Duration(2) * time.Minute return &HTTPClient{ - Client: hc, - RetryConfig: &RetryConfig{ - MaxRetries: 4, - CheckForRetry: retryNetworkAndHTTPErrors( - http.StatusServiceUnavailable, - ), - ExpBackoffFactor: 0.5, - MaxDelay: &twoMinutes, - }, + Client: hc, + RetryConfig: defaultRetryConfig(), + } +} + +// WithRetryConfig creates a ClientOption that can be used to configure HTTP retries. +// +// The option can be passed into NewApp() and is propagated to service clients. +// If this option is provided with a nil RetryConfig, retries are disabled. +func WithRetryConfig(retryConfig *RetryConfig) option.ClientOption { + return &withRetryConfigOption{ + EmbeddableAdapter: &internaloption.EmbeddableAdapter{}, + retryConfig: retryConfig, } } @@ -371,6 +381,41 @@ type RetryConfig struct { MaxDelay *time.Duration } +type withRetryConfigOption struct { + *internaloption.EmbeddableAdapter + retryConfig *RetryConfig +} + +func (w *withRetryConfigOption) getRetryConfig() (*RetryConfig, bool) { + return w.retryConfig, true +} + +type retryConfigOption interface { + getRetryConfig() (*RetryConfig, bool) +} + +func retryConfigFromOptions(opts ...option.ClientOption) (*RetryConfig, bool) { + for idx := len(opts) - 1; idx >= 0; idx-- { + if rcOpt, ok := opts[idx].(retryConfigOption); ok { + return rcOpt.getRetryConfig() + } + } + + return nil, false +} + +func defaultRetryConfig() *RetryConfig { + twoMinutes := time.Duration(2) * time.Minute + return &RetryConfig{ + MaxRetries: 4, + CheckForRetry: retryNetworkAndHTTPErrors( + http.StatusServiceUnavailable, + ), + ExpBackoffFactor: 0.5, + MaxDelay: &twoMinutes, + } +} + // RetryCondition determines if an HTTP request should be retried depending on its last outcome. type RetryCondition func(resp *http.Response, networkErr error) bool diff --git a/internal/http_client_test.go b/internal/http_client_test.go index 22b498cf..c81bafe2 100644 --- a/internal/http_client_test.go +++ b/internal/http_client_test.go @@ -693,6 +693,38 @@ func TestNewHTTPClient(t *testing.T) { } } +func TestNewHTTPClientWithRetryConfigOption(t *testing.T) { + wantRetry := &RetryConfig{ + MaxRetries: 1, + ExpBackoffFactor: 1.25, + } + client, _, err := NewHTTPClient( + context.Background(), + tokenSourceOpt, + WithRetryConfig(wantRetry), + ) + if err != nil { + t.Fatal(err) + } + if client.RetryConfig != wantRetry { + t.Errorf("NewHTTPClient().RetryConfig = %p; want = %p", client.RetryConfig, wantRetry) + } +} + +func TestNewHTTPClientWithRetryConfigOptionNil(t *testing.T) { + client, _, err := NewHTTPClient( + context.Background(), + tokenSourceOpt, + WithRetryConfig(nil), + ) + if err != nil { + t.Fatal(err) + } + if client.RetryConfig != nil { + t.Errorf("NewHTTPClient().RetryConfig = %v; want = nil", client.RetryConfig) + } +} + func TestNewHTTPClientRetryOnNetworkErrors(t *testing.T) { client, _, err := NewHTTPClient(context.Background(), tokenSourceOpt) if err != nil { diff --git a/messaging/messaging.go b/messaging/messaging.go index 2eda22a9..96f5f940 100644 --- a/messaging/messaging.go +++ b/messaging/messaging.go @@ -28,7 +28,6 @@ import ( "time" "firebase.google.com/go/v4/internal" - "google.golang.org/api/transport" ) const ( @@ -908,7 +907,7 @@ func NewClient(ctx context.Context, c *internal.MessagingConfig) (*Client, error return nil, errors.New("project ID is required to access Firebase Cloud Messaging client") } - hc, messagingEndpoint, err := transport.NewHTTPClient(ctx, c.Opts...) + baseHTTPClient, messagingEndpoint, err := internal.NewHTTPClient(ctx, c.Opts...) if err != nil { return nil, err } @@ -921,8 +920,8 @@ func NewClient(ctx context.Context, c *internal.MessagingConfig) (*Client, error } return &Client{ - fcmClient: newFCMClient(hc, c, messagingEndpoint, batchEndpoint), - iidClient: newIIDClient(hc, c), + fcmClient: newFCMClient(baseHTTPClient, c, messagingEndpoint, batchEndpoint), + iidClient: newIIDClient(baseHTTPClient, c), }, nil } @@ -934,8 +933,8 @@ type fcmClient struct { httpClient *internal.HTTPClient } -func newFCMClient(hc *http.Client, conf *internal.MessagingConfig, messagingEndpoint string, batchEndpoint string) *fcmClient { - client := internal.WithDefaultRetryConfig(hc) +func newFCMClient(base *internal.HTTPClient, conf *internal.MessagingConfig, messagingEndpoint string, batchEndpoint string) *fcmClient { + client := cloneHTTPClient(base) client.CreateErrFn = handleFCMError version := fmt.Sprintf("fire-admin-go/%s", conf.Version) @@ -954,6 +953,11 @@ func newFCMClient(hc *http.Client, conf *internal.MessagingConfig, messagingEndp } } +func cloneHTTPClient(client *internal.HTTPClient) *internal.HTTPClient { + clone := *client + return &clone +} + // Send sends a Message to Firebase Cloud Messaging. // // The Message must specify exactly one of Token, Topic and Condition fields. FCM will diff --git a/messaging/messaging_test.go b/messaging/messaging_test.go index 0cb72065..d6984d51 100644 --- a/messaging/messaging_test.go +++ b/messaging/messaging_test.go @@ -1360,6 +1360,49 @@ func TestSendWithCustomEndpoint(t *testing.T) { } } +func TestNewClientWithRetryConfigOption(t *testing.T) { + ctx := context.Background() + + customRetry := &internal.RetryConfig{ + MaxRetries: 2, + ExpBackoffFactor: 0.25, + } + + conf := *testMessagingConfig + conf.Opts = append(conf.Opts, internal.WithRetryConfig(customRetry)) + + client, err := NewClient(ctx, &conf) + if err != nil { + t.Fatal(err) + } + + if client.fcmClient.httpClient.RetryConfig != customRetry { + t.Errorf("fcm retry config = %p; want = %p", client.fcmClient.httpClient.RetryConfig, customRetry) + } + if client.iidClient.httpClient.RetryConfig != customRetry { + t.Errorf("iid retry config = %p; want = %p", client.iidClient.httpClient.RetryConfig, customRetry) + } +} + +func TestNewClientWithNilRetryConfigOption(t *testing.T) { + ctx := context.Background() + + conf := *testMessagingConfig + conf.Opts = append(conf.Opts, internal.WithRetryConfig(nil)) + + client, err := NewClient(ctx, &conf) + if err != nil { + t.Fatal(err) + } + + if client.fcmClient.httpClient.RetryConfig != nil { + t.Errorf("fcm retry config = %v; want = nil", client.fcmClient.httpClient.RetryConfig) + } + if client.iidClient.httpClient.RetryConfig != nil { + t.Errorf("iid retry config = %v; want = nil", client.iidClient.httpClient.RetryConfig) + } +} + func TestSendDryRun(t *testing.T) { var tr *http.Request var b []byte diff --git a/messaging/topic_mgt.go b/messaging/topic_mgt.go index 0e7e9d0c..a91c09ce 100644 --- a/messaging/topic_mgt.go +++ b/messaging/topic_mgt.go @@ -63,8 +63,8 @@ type iidClient struct { httpClient *internal.HTTPClient } -func newIIDClient(hc *http.Client, conf *internal.MessagingConfig) *iidClient { - client := internal.WithDefaultRetryConfig(hc) +func newIIDClient(base *internal.HTTPClient, conf *internal.MessagingConfig) *iidClient { + client := cloneHTTPClient(base) client.CreateErrFn = handleIIDError client.Opts = []internal.HTTPOption{ internal.WithHeader("access_token_auth", "true"), diff --git a/retry_options.go b/retry_options.go new file mode 100644 index 00000000..61006c73 --- /dev/null +++ b/retry_options.go @@ -0,0 +1,31 @@ +// Copyright 2017 Google LLC All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package firebase + +import ( + "firebase.google.com/go/v4/internal" + "google.golang.org/api/option" +) + +// RetryConfig specifies how Admin SDK HTTP clients should retry failing requests. +type RetryConfig = internal.RetryConfig + +// WithRetryConfig creates a ClientOption that configures HTTP retry behavior. +// +// Pass this option to NewApp() to configure retries for service clients. +// If set with a nil RetryConfig, retries are disabled. +func WithRetryConfig(retryConfig *RetryConfig) option.ClientOption { + return internal.WithRetryConfig(retryConfig) +} From f206318ba208022393b6086bb056700b811913ef Mon Sep 17 00:00:00 2001 From: "amin.paydar" Date: Tue, 5 May 2026 16:18:42 +0330 Subject: [PATCH 2/2] fix(fcm): preserve inherited HTTP options when cloning clients Clone internal HTTP clients with copied slice/pointer fields and append messaging-specific headers instead of overwriting options. This keeps future defaults from internal.NewHTTPClient intact for both FCM and IID clients. --- internal/http_client.go | 24 +++++++++++++++++++++ internal/http_client_test.go | 41 +++++++++++++++++++++++++++++++++++ messaging/messaging.go | 11 +++------- messaging/messaging_test.go | 42 ++++++++++++++++++++++++++++++++---- messaging/topic_mgt.go | 6 +++--- 5 files changed, 109 insertions(+), 15 deletions(-) diff --git a/internal/http_client.go b/internal/http_client.go index 26ceff10..2a5b22b2 100644 --- a/internal/http_client.go +++ b/internal/http_client.go @@ -88,6 +88,30 @@ func WithDefaultRetryConfig(hc *http.Client) *HTTPClient { } } +// CloneHTTPClient returns a copy of the given HTTPClient. +// +// Slice and pointer fields are copied to avoid accidental cross-client mutations. +func CloneHTTPClient(client *HTTPClient) *HTTPClient { + if client == nil { + return nil + } + + clone := *client + if client.Opts != nil { + clone.Opts = append([]HTTPOption{}, client.Opts...) + } + if client.RetryConfig != nil { + retryConfig := *client.RetryConfig + if client.RetryConfig.MaxDelay != nil { + maxDelay := *client.RetryConfig.MaxDelay + retryConfig.MaxDelay = &maxDelay + } + clone.RetryConfig = &retryConfig + } + + return &clone +} + // WithRetryConfig creates a ClientOption that can be used to configure HTTP retries. // // The option can be passed into NewApp() and is propagated to service clients. diff --git a/internal/http_client_test.go b/internal/http_client_test.go index c81bafe2..710afe98 100644 --- a/internal/http_client_test.go +++ b/internal/http_client_test.go @@ -725,6 +725,47 @@ func TestNewHTTPClientWithRetryConfigOptionNil(t *testing.T) { } } +func TestCloneHTTPClient(t *testing.T) { + delay := 5 * time.Second + original := &HTTPClient{ + Client: &http.Client{}, + RetryConfig: &RetryConfig{ + MaxRetries: 3, + CheckForRetry: retryNetworkAndHTTPErrors(http.StatusServiceUnavailable), + ExpBackoffFactor: 0.5, + MaxDelay: &delay, + }, + Opts: []HTTPOption{ + WithHeader("X-Test", "value"), + }, + } + + cloned := CloneHTTPClient(original) + if cloned == original { + t.Fatalf("CloneHTTPClient() returned the original instance") + } + if len(cloned.Opts) != len(original.Opts) { + t.Fatalf("len(Opts) = %d; want = %d", len(cloned.Opts), len(original.Opts)) + } + if cloned.RetryConfig == original.RetryConfig { + t.Errorf("RetryConfig pointer should be copied") + } + if cloned.RetryConfig.MaxDelay == original.RetryConfig.MaxDelay { + t.Errorf("RetryConfig.MaxDelay pointer should be copied") + } + + cloned.Opts = append(cloned.Opts, WithHeader("X-Test-2", "value-2")) + if len(original.Opts) != 1 { + t.Errorf("len(original.Opts) = %d; want = 1", len(original.Opts)) + } + + newDelay := 10 * time.Second + cloned.RetryConfig.MaxDelay = &newDelay + if *original.RetryConfig.MaxDelay != delay { + t.Errorf("original RetryConfig.MaxDelay = %v; want = %v", *original.RetryConfig.MaxDelay, delay) + } +} + func TestNewHTTPClientRetryOnNetworkErrors(t *testing.T) { client, _, err := NewHTTPClient(context.Background(), tokenSourceOpt) if err != nil { diff --git a/messaging/messaging.go b/messaging/messaging.go index 96f5f940..be0bef1f 100644 --- a/messaging/messaging.go +++ b/messaging/messaging.go @@ -934,15 +934,15 @@ type fcmClient struct { } func newFCMClient(base *internal.HTTPClient, conf *internal.MessagingConfig, messagingEndpoint string, batchEndpoint string) *fcmClient { - client := cloneHTTPClient(base) + client := internal.CloneHTTPClient(base) client.CreateErrFn = handleFCMError version := fmt.Sprintf("fire-admin-go/%s", conf.Version) - client.Opts = []internal.HTTPOption{ + client.Opts = append(client.Opts, internal.WithHeader(apiFormatVersionHeader, apiFormatVersion), internal.WithHeader(firebaseClientHeader, version), internal.WithHeader("x-goog-api-client", internal.GetMetricsHeader(conf.Version)), - } + ) return &fcmClient{ fcmEndpoint: messagingEndpoint, @@ -953,11 +953,6 @@ func newFCMClient(base *internal.HTTPClient, conf *internal.MessagingConfig, mes } } -func cloneHTTPClient(client *internal.HTTPClient) *internal.HTTPClient { - clone := *client - return &clone -} - // Send sends a Message to Firebase Cloud Messaging. // // The Message must specify exactly one of Token, Topic and Condition fields. FCM will diff --git a/messaging/messaging_test.go b/messaging/messaging_test.go index d6984d51..f081404a 100644 --- a/messaging/messaging_test.go +++ b/messaging/messaging_test.go @@ -1376,11 +1376,17 @@ func TestNewClientWithRetryConfigOption(t *testing.T) { t.Fatal(err) } - if client.fcmClient.httpClient.RetryConfig != customRetry { - t.Errorf("fcm retry config = %p; want = %p", client.fcmClient.httpClient.RetryConfig, customRetry) + if client.fcmClient.httpClient.RetryConfig == nil { + t.Fatal("fcm retry config = nil; want non-nil") } - if client.iidClient.httpClient.RetryConfig != customRetry { - t.Errorf("iid retry config = %p; want = %p", client.iidClient.httpClient.RetryConfig, customRetry) + if client.iidClient.httpClient.RetryConfig == nil { + t.Fatal("iid retry config = nil; want non-nil") + } + if !reflect.DeepEqual(client.fcmClient.httpClient.RetryConfig, customRetry) { + t.Errorf("fcm retry config = %#v; want = %#v", client.fcmClient.httpClient.RetryConfig, customRetry) + } + if !reflect.DeepEqual(client.iidClient.httpClient.RetryConfig, customRetry) { + t.Errorf("iid retry config = %#v; want = %#v", client.iidClient.httpClient.RetryConfig, customRetry) } } @@ -1403,6 +1409,34 @@ func TestNewClientWithNilRetryConfigOption(t *testing.T) { } } +func TestMessagingClientPreservesBaseHTTPOptions(t *testing.T) { + base := &internal.HTTPClient{ + Client: &http.Client{}, + Opts: []internal.HTTPOption{ + internal.WithHeader("X-Base-Header", "base"), + }, + } + + conf := &internal.MessagingConfig{ + ProjectID: "test-project", + Version: "test-version", + } + + fcm := newFCMClient(base, conf, defaultMessagingEndpoint, defaultBatchEndpoint) + iid := newIIDClient(base, conf) + + if len(base.Opts) != 1 { + t.Fatalf("len(base.Opts) = %d; want = 1", len(base.Opts)) + } + + if len(fcm.httpClient.Opts) != 4 { + t.Errorf("len(fcm.httpClient.Opts) = %d; want = 4", len(fcm.httpClient.Opts)) + } + if len(iid.httpClient.Opts) != 3 { + t.Errorf("len(iid.httpClient.Opts) = %d; want = 3", len(iid.httpClient.Opts)) + } +} + func TestSendDryRun(t *testing.T) { var tr *http.Request var b []byte diff --git a/messaging/topic_mgt.go b/messaging/topic_mgt.go index a91c09ce..d49b2bf2 100644 --- a/messaging/topic_mgt.go +++ b/messaging/topic_mgt.go @@ -64,12 +64,12 @@ type iidClient struct { } func newIIDClient(base *internal.HTTPClient, conf *internal.MessagingConfig) *iidClient { - client := cloneHTTPClient(base) + client := internal.CloneHTTPClient(base) client.CreateErrFn = handleIIDError - client.Opts = []internal.HTTPOption{ + client.Opts = append(client.Opts, internal.WithHeader("access_token_auth", "true"), internal.WithHeader("x-goog-api-client", internal.GetMetricsHeader(conf.Version)), - } + ) return &iidClient{ iidEndpoint: iidEndpoint, httpClient: client,