From b3e26f9083a5fed2b6d5021af599b80e1bd662ca Mon Sep 17 00:00:00 2001 From: "amin.paydar" Date: Wed, 6 May 2026 11:27:52 +0330 Subject: [PATCH] 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 | 91 +++++++++++++++++++++++++++++++----- internal/http_client_test.go | 73 +++++++++++++++++++++++++++++ messaging/messaging.go | 15 +++--- messaging/messaging_test.go | 77 ++++++++++++++++++++++++++++++ messaging/topic_mgt.go | 8 ++-- retry_options.go | 31 ++++++++++++ 6 files changed, 272 insertions(+), 23 deletions(-) create mode 100644 retry_options.go diff --git a/internal/http_client.go b/internal/http_client.go index 9d1257bc..2a5b22b2 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,44 @@ 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(), + } +} + +// 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. +// 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 +405,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..710afe98 100644 --- a/internal/http_client_test.go +++ b/internal/http_client_test.go @@ -693,6 +693,79 @@ 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 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 67b94f66..94e138cd 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 ( @@ -910,7 +909,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 } @@ -923,8 +922,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 } @@ -936,16 +935,16 @@ 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 := 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, diff --git a/messaging/messaging_test.go b/messaging/messaging_test.go index 5ba548af..9fcab766 100644 --- a/messaging/messaging_test.go +++ b/messaging/messaging_test.go @@ -1364,6 +1364,83 @@ 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 == nil { + t.Fatal("fcm retry config = nil; want non-nil") + } + 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) + } +} + +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 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 0e7e9d0c..d49b2bf2 100644 --- a/messaging/topic_mgt.go +++ b/messaging/topic_mgt.go @@ -63,13 +63,13 @@ 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 := 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, 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) +}