Report a rate-limited bank sync as a wait, and name real failures
Two of three banks were only pacing us, yet the dashboard demanded attention, printed four nested wrappers and a nanosecond UTC deadline, and the scheduler retried hourly into a refusal whose end time the bank had already given. A rate limit now carries its retry time as data: Status.SyncRetryAt is set when every failure is self-clearing, the connection reports rate_limited with that deadline, the dashboard says synchronization resumes by itself and renders the time in the browser's zone, and the scheduler sleeps until the deadline instead of spending hourly session checks. Sync now still tries immediately. The third bank's "transaction retrieval failed" hid its cause. Provider failures Finance Duck determines itself are typed as banking.ProviderError, so an unreachable provider, a timeout or an unusable response, such as a booked transaction without a booking date, is reported instead of the opaque fallback. Provider response text still never reaches the message.
This commit is contained in:
+9
-4
@@ -29,9 +29,12 @@ type Settings struct {
|
||||
ClassifyOnImport bool `json:"classify_on_import"`
|
||||
}
|
||||
type Status struct {
|
||||
SyncError string `json:"sync_error"`
|
||||
IndexError string `json:"index_error"`
|
||||
LastSync string `json:"last_sync"`
|
||||
SyncError string `json:"sync_error"`
|
||||
IndexError string `json:"index_error"`
|
||||
LastSync string `json:"last_sync"`
|
||||
// SyncRetryAt is set only when every sync failure is a bank rate limit that
|
||||
// clears on its own; it is the earliest time an automatic retry is allowed.
|
||||
SyncRetryAt string `json:"sync_retry_at,omitempty"`
|
||||
BankingConfigured bool `json:"banking_configured"`
|
||||
AIConfigured bool `json:"ai_configured"`
|
||||
}
|
||||
@@ -49,6 +52,7 @@ type operational struct {
|
||||
Sessions []banking.Session `json:"sessions"`
|
||||
LastSync string `json:"last_sync"`
|
||||
SyncError string `json:"sync_error"`
|
||||
SyncRetryAt string `json:"sync_retry_at,omitempty"`
|
||||
Consents map[string]Consent `json:"consents"`
|
||||
AccountSync map[string]string `json:"account_sync"`
|
||||
BankingScope string `json:"banking_scope"`
|
||||
@@ -166,7 +170,8 @@ func (a *App) snapshot(ctx context.Context) (State, error) {
|
||||
a.indexError = ""
|
||||
}
|
||||
}
|
||||
return State{Data: d, Revision: rev, Settings: a.settings, Sessions: copySessions(a.ops.Sessions), CallbackURL: a.callbackURL, BankingAppID: a.bankingSettings.AppID, Connections: a.connections(d), Status: Status{SyncError: a.ops.SyncError, LastSync: a.ops.LastSync, IndexError: a.indexError, BankingConfigured: a.bank != nil, AIConfigured: a.classifier.APIKey != ""}}, nil
|
||||
status := Status{SyncError: a.ops.SyncError, SyncRetryAt: a.ops.SyncRetryAt, LastSync: a.ops.LastSync, IndexError: a.indexError, BankingConfigured: a.bank != nil, AIConfigured: a.classifier.APIKey != ""}
|
||||
return State{Data: d, Revision: rev, Settings: a.settings, Sessions: copySessions(a.ops.Sessions), CallbackURL: a.callbackURL, BankingAppID: a.bankingSettings.AppID, Connections: a.connections(d), Status: status}, nil
|
||||
}
|
||||
func (a *App) Snapshot(ctx context.Context) (State, error) {
|
||||
a.mu.Lock()
|
||||
|
||||
@@ -31,6 +31,7 @@ func (a *App) clearBankingSessions(scope string) {
|
||||
a.ops.AccountSync = make(map[string]string)
|
||||
a.ops.LastSync = ""
|
||||
a.ops.SyncError = ""
|
||||
a.ops.SyncRetryAt = ""
|
||||
a.ops.BankingScope = scope
|
||||
}
|
||||
|
||||
|
||||
@@ -27,6 +27,8 @@ type Consent struct {
|
||||
HistoryMonths int `json:"history_months"`
|
||||
Error string `json:"error,omitempty"`
|
||||
NeedsReconnect bool `json:"needs_reconnect"`
|
||||
// RetryAt is the bank's own retry time while it rate limits this consent.
|
||||
RetryAt string `json:"retry_at,omitempty"`
|
||||
}
|
||||
type Connection struct {
|
||||
AccountID string `json:"account_id"`
|
||||
@@ -37,6 +39,7 @@ type Connection struct {
|
||||
Status string `json:"status"`
|
||||
ValidUntil string `json:"valid_until"`
|
||||
Error string `json:"error"`
|
||||
RetryAt string `json:"retry_at,omitempty"`
|
||||
}
|
||||
|
||||
// psuType keeps legacy consents, which predate the choice, on the personal
|
||||
@@ -79,6 +82,7 @@ func (a *App) connections(d domain.Dataset) []Connection {
|
||||
}
|
||||
c.ValidUntil = session.ValidUntil
|
||||
c.Error = meta.Error
|
||||
c.RetryAt = meta.RetryAt
|
||||
c.Status = "connected"
|
||||
expiry, err := time.Parse(time.RFC3339, session.ValidUntil)
|
||||
if meta.NeedsReconnect || err != nil || !expiry.After(time.Now()) {
|
||||
@@ -86,6 +90,9 @@ func (a *App) connections(d domain.Dataset) []Connection {
|
||||
if c.Error == "" {
|
||||
c.Error = "Bank consent expired; reconnect to resume automatic imports"
|
||||
}
|
||||
} else if meta.RetryAt != "" {
|
||||
// A rate limit is the bank pacing us, not a broken connection.
|
||||
c.Status = "rate_limited"
|
||||
} else if meta.Error != "" {
|
||||
c.Status = "error"
|
||||
}
|
||||
|
||||
+85
-14
@@ -605,10 +605,12 @@ func (a *App) Balances(ctx context.Context, id string) ([]banking.Balance, error
|
||||
|
||||
// Only typed, locally generated errors are safe to expose; provider errors may
|
||||
// wrap private response data even when their underlying cause is recognizable.
|
||||
// The fallback is a last resort: an unnamed cause leaves an operator with
|
||||
// nothing to act on.
|
||||
func bankFailure(err error, fallback string) error {
|
||||
var background *banking.BackgroundQuotaError
|
||||
if errors.As(err, &background) {
|
||||
return fmt.Errorf("Enable Banking: %w", background)
|
||||
return background
|
||||
}
|
||||
var limited *ratelimit.RateLimitError
|
||||
if errors.As(err, &limited) {
|
||||
@@ -625,9 +627,25 @@ func bankFailure(err error, fallback string) error {
|
||||
if errors.As(err, &consent) {
|
||||
return consent
|
||||
}
|
||||
var provider *banking.ProviderError
|
||||
if errors.As(err, &provider) {
|
||||
return provider
|
||||
}
|
||||
return errors.New(fallback)
|
||||
}
|
||||
|
||||
// syncRetryAt reports the bank's own retry time for a failure that clears
|
||||
// itself. A rate limit without a usable deadline is not treated as waiting:
|
||||
// nothing would ever announce that it had expired.
|
||||
func syncRetryAt(err error) (time.Time, bool) {
|
||||
var limited *ratelimit.RateLimitError
|
||||
if !errors.As(err, &limited) {
|
||||
return time.Time{}, false
|
||||
}
|
||||
at := limited.RetryAt()
|
||||
return at, !at.IsZero()
|
||||
}
|
||||
|
||||
func (a *App) Sync(ctx context.Context) (State, error) {
|
||||
a.mu.Lock()
|
||||
defer a.mu.Unlock()
|
||||
@@ -639,6 +657,10 @@ func (a *App) Sync(ctx context.Context) (State, error) {
|
||||
return State{}, err
|
||||
}
|
||||
var failures []string
|
||||
// waitUntil is the earliest time the banks themselves allow a retry, used
|
||||
// only while every failure is such a self-clearing rate limit.
|
||||
var waitUntil time.Time
|
||||
waiting := true
|
||||
for i := range a.ops.Sessions {
|
||||
connectAccounts(&s.Data, &a.ops.Sessions[i], false)
|
||||
}
|
||||
@@ -684,6 +706,15 @@ func (a *App) Sync(ctx context.Context) (State, error) {
|
||||
}
|
||||
if e != nil {
|
||||
meta.Error = bankFailure(e, "bank connection unavailable; retry synchronization").Error()
|
||||
meta.RetryAt = ""
|
||||
if at, ok := syncRetryAt(e); ok {
|
||||
meta.RetryAt = at.UTC().Format(time.RFC3339)
|
||||
if waitUntil.IsZero() || at.Before(waitUntil) {
|
||||
waitUntil = at
|
||||
}
|
||||
} else {
|
||||
waiting = false
|
||||
}
|
||||
meta.NeedsReconnect = errors.Is(e, banking.ErrReconnect)
|
||||
a.ops.Consents[session.ID] = meta
|
||||
failures = append(failures, meta.Institution+": "+meta.Error)
|
||||
@@ -691,6 +722,7 @@ func (a *App) Sync(ctx context.Context) (State, error) {
|
||||
continue
|
||||
}
|
||||
meta.Error = ""
|
||||
meta.RetryAt = ""
|
||||
meta.NeedsReconnect = false
|
||||
a.ops.Consents[session.ID] = meta
|
||||
a.ops.Sessions[i].ValidUntil = current.ValidUntil
|
||||
@@ -712,9 +744,11 @@ func (a *App) Sync(ctx context.Context) (State, error) {
|
||||
}
|
||||
if !validAccounts[account.ID] {
|
||||
failures = append(failures, account.DisplayName+": bank connection unavailable")
|
||||
waiting = false
|
||||
if sessionID != "" {
|
||||
meta := a.ops.Consents[sessionID]
|
||||
meta.Error = banking.ErrReconnect.Error()
|
||||
meta.RetryAt = ""
|
||||
meta.NeedsReconnect = true
|
||||
a.ops.Consents[sessionID] = meta
|
||||
}
|
||||
@@ -731,6 +765,15 @@ func (a *App) Sync(ctx context.Context) (State, error) {
|
||||
if e != nil {
|
||||
meta := a.ops.Consents[sessionID]
|
||||
meta.Error = bankFailure(e, "transaction retrieval failed; retry synchronization").Error()
|
||||
meta.RetryAt = ""
|
||||
if at, ok := syncRetryAt(e); ok {
|
||||
meta.RetryAt = at.UTC().Format(time.RFC3339)
|
||||
if waitUntil.IsZero() || at.Before(waitUntil) {
|
||||
waitUntil = at
|
||||
}
|
||||
} else {
|
||||
waiting = false
|
||||
}
|
||||
meta.NeedsReconnect = meta.NeedsReconnect || errors.Is(e, banking.ErrReconnect)
|
||||
a.ops.Consents[sessionID] = meta
|
||||
failures = append(failures, account.DisplayName+": "+meta.Error)
|
||||
@@ -739,6 +782,7 @@ func (a *App) Sync(ctx context.Context) (State, error) {
|
||||
result, e := a.importFacts(ctx, s, facts)
|
||||
if e != nil {
|
||||
failures = append(failures, account.DisplayName+": "+e.Error())
|
||||
waiting = false
|
||||
s, err = a.snapshot(ctx)
|
||||
if err != nil {
|
||||
return State{}, err
|
||||
@@ -749,14 +793,47 @@ func (a *App) Sync(ctx context.Context) (State, error) {
|
||||
a.ops.AccountSync[account.ID] = now.Format(time.RFC3339)
|
||||
}
|
||||
a.ops.SyncError = strings.Join(failures, "; ")
|
||||
a.ops.SyncRetryAt = ""
|
||||
if len(failures) == 0 {
|
||||
a.ops.LastSync = now.Format(time.RFC3339)
|
||||
} else if waiting && !waitUntil.IsZero() {
|
||||
// Every bank named its own retry time: this is a wait, not a fault.
|
||||
a.ops.SyncRetryAt = waitUntil.UTC().Format(time.RFC3339)
|
||||
}
|
||||
if err = a.saveOps(); err != nil {
|
||||
return State{}, err
|
||||
}
|
||||
return a.snapshot(ctx)
|
||||
}
|
||||
|
||||
// syncSchedule decides whether an automatic sync may run now, and how long to
|
||||
// wait otherwise. While a bank has named its own retry time, waiting is the
|
||||
// only useful action: retrying earlier spends session-status calls on a refusal
|
||||
// that is already known. A manual request always proceeds.
|
||||
func syncSchedule(now time.Time, ops operational, force bool) (time.Duration, bool) {
|
||||
if retry, err := time.Parse(time.RFC3339, ops.SyncRetryAt); err == nil && !force && now.Before(retry) {
|
||||
return min(retry.Sub(now)+time.Minute, time.Hour), false
|
||||
}
|
||||
last, err := time.Parse(time.RFC3339, ops.LastSync)
|
||||
if force || err != nil || ops.SyncError != "" || now.Sub(last) >= 24*time.Hour {
|
||||
return 0, true
|
||||
}
|
||||
return time.Minute, false
|
||||
}
|
||||
|
||||
// syncBackoff spaces the next attempt after a sync. A failure with a known bank
|
||||
// retry time waits for it; other failures retry hourly so transient provider
|
||||
// problems clear without waiting a day, while bounding unattended traffic.
|
||||
func syncBackoff(now time.Time, ops operational) time.Duration {
|
||||
if ops.SyncError == "" {
|
||||
return 24 * time.Hour
|
||||
}
|
||||
if retry, err := time.Parse(time.RFC3339, ops.SyncRetryAt); err == nil && now.Before(retry) {
|
||||
return min(retry.Sub(now)+time.Minute, 24*time.Hour)
|
||||
}
|
||||
return time.Hour
|
||||
}
|
||||
|
||||
func (a *App) RunScheduler(ctx context.Context) {
|
||||
timer := time.NewTimer(time.Minute)
|
||||
defer timer.Stop()
|
||||
@@ -771,25 +848,19 @@ func (a *App) RunScheduler(ctx context.Context) {
|
||||
}
|
||||
a.mu.Lock()
|
||||
configured := a.bank != nil
|
||||
last, err := time.Parse(time.RFC3339, a.ops.LastSync)
|
||||
failed := a.ops.SyncError != ""
|
||||
wait, due := syncSchedule(time.Now(), a.ops, force)
|
||||
a.mu.Unlock()
|
||||
due := force || err != nil || failed || time.Since(last) >= 24*time.Hour
|
||||
if !configured || !due {
|
||||
timer.Reset(time.Minute)
|
||||
if wait <= 0 {
|
||||
wait = time.Minute
|
||||
}
|
||||
timer.Reset(wait)
|
||||
continue
|
||||
}
|
||||
a.Sync(ctx)
|
||||
a.mu.Lock()
|
||||
failed = a.ops.SyncError != ""
|
||||
wait = syncBackoff(time.Now(), a.ops)
|
||||
a.mu.Unlock()
|
||||
if failed {
|
||||
// A failed sync leaves its persisted error banner behind. Retry
|
||||
// hourly so transient provider failures clear without waiting a
|
||||
// day, while bounding unattended traffic toward the provider.
|
||||
timer.Reset(time.Hour)
|
||||
} else {
|
||||
timer.Reset(24 * time.Hour)
|
||||
}
|
||||
timer.Reset(wait)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -273,7 +273,7 @@ func TestSyncMissingMembershipStillRejectsAccount(t *testing.T) {
|
||||
}
|
||||
|
||||
func TestSyncTransactionFailuresPreserveProgressAndSafeErrors(t *testing.T) {
|
||||
for _, scenario := range []string{"rate limit", "background rate limit", "reconnect", "private response"} {
|
||||
for _, scenario := range []string{"rate limit", "background rate limit", "reconnect", "private response", "unusable response"} {
|
||||
t.Run(scenario, func(t *testing.T) {
|
||||
a, s, b := backfillApp(t)
|
||||
s = seed(t, a, s)
|
||||
@@ -292,6 +292,9 @@ func TestSyncTransactionFailuresPreserveProgressAndSafeErrors(t *testing.T) {
|
||||
b.fetchErr = fmt.Errorf("private provider response: %w", banking.ErrReconnect)
|
||||
case "private response":
|
||||
b.fetchErr = errors.New("private provider response")
|
||||
case "unusable response":
|
||||
unusable := &banking.ProviderError{Detail: "a booked transaction has no valid booking date"}
|
||||
b.fetchErr = fmt.Errorf("private provider response: %w", unusable)
|
||||
}
|
||||
failed, err := a.Sync(context.Background())
|
||||
if err != nil {
|
||||
@@ -313,6 +316,43 @@ func TestSyncTransactionFailuresPreserveProgressAndSafeErrors(t *testing.T) {
|
||||
if !reflect.DeepEqual(s.Data, failed.Data) || !reflect.DeepEqual(cursors, a.ops.AccountSync) || a.ops.LastSync != old {
|
||||
t.Fatal("failed retrieval imported partial data or advanced synchronization")
|
||||
}
|
||||
rateLimited := strings.Contains(scenario, "rate limit")
|
||||
// A bank that named its own retry time is a wait, not a fault: the
|
||||
// UI and the scheduler both rely on this distinction.
|
||||
if (failed.Status.SyncRetryAt != "") != rateLimited {
|
||||
t.Fatalf("waiting state is wrong for %s: retry at %q", scenario, failed.Status.SyncRetryAt)
|
||||
}
|
||||
for _, connection := range failed.Connections {
|
||||
if connection.Status == "local" {
|
||||
continue
|
||||
}
|
||||
want := "error"
|
||||
switch {
|
||||
case rateLimited:
|
||||
want = "rate_limited"
|
||||
case scenario == "reconnect":
|
||||
want = "reconnect_required"
|
||||
}
|
||||
if connection.Status != want || (connection.RetryAt != "") != rateLimited {
|
||||
t.Fatalf("connection reported %q with retry %q, want %q", connection.Status, connection.RetryAt, want)
|
||||
}
|
||||
}
|
||||
if rateLimited {
|
||||
at, e := time.Parse(time.RFC3339, failed.Status.SyncRetryAt)
|
||||
if e != nil || !at.After(time.Now()) {
|
||||
t.Fatalf("unusable retry deadline %q: %v", failed.Status.SyncRetryAt, e)
|
||||
}
|
||||
// Whole seconds, once: operators read this message.
|
||||
if !strings.Contains(meta.Error, at.UTC().Format(time.RFC3339)) || strings.Count(meta.Error, "429") != 1 {
|
||||
t.Fatalf("rate limit message is not legible: %q", meta.Error)
|
||||
}
|
||||
}
|
||||
if scenario == "unusable response" && (!strings.Contains(meta.Error, "cannot use") || !strings.Contains(meta.Error, "booking date")) {
|
||||
t.Fatalf("unusable provider response was reduced to an opaque failure: %q", meta.Error)
|
||||
}
|
||||
if scenario == "private response" && !strings.Contains(meta.Error, "transaction retrieval failed") {
|
||||
t.Fatalf("unrecognized failure lost its fallback: %q", meta.Error)
|
||||
}
|
||||
b.fetchErr = nil
|
||||
recovered, err := a.Sync(context.Background())
|
||||
if err != nil || recovered.Status.SyncError != "" || a.ops.Consents["current"].Error != "" || a.ops.Consents["current"].NeedsReconnect {
|
||||
@@ -321,3 +361,45 @@ func TestSyncTransactionFailuresPreserveProgressAndSafeErrors(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// The scheduler must not spend session-status calls on a refusal the bank has
|
||||
// already scheduled, and must not sit on a deadline that has passed.
|
||||
func TestSyncSchedulingRespectsTheBanksOwnRetryTime(t *testing.T) {
|
||||
// Persisted deadlines carry whole seconds; compare against the same grid.
|
||||
now := time.Now().UTC().Truncate(time.Second)
|
||||
stale := now.Add(-48 * time.Hour).Format(time.RFC3339)
|
||||
waiting := operational{LastSync: stale, SyncError: "N26: rate limited", SyncRetryAt: now.Add(3 * time.Hour).Format(time.RFC3339)}
|
||||
broken := operational{LastSync: stale, SyncError: "Trade Republic: retrieval failed"}
|
||||
healthy := operational{LastSync: now.Format(time.RFC3339)}
|
||||
elapsed := operational{LastSync: stale, SyncError: waiting.SyncError, SyncRetryAt: now.Add(-time.Minute).Format(time.RFC3339)}
|
||||
cases := []struct {
|
||||
name string
|
||||
ops operational
|
||||
force bool
|
||||
wait time.Duration
|
||||
due bool
|
||||
}{
|
||||
{"waiting for the bank", waiting, false, time.Hour, false},
|
||||
{"manual sync during a wait", waiting, true, 0, true},
|
||||
{"deadline elapsed", elapsed, false, 0, true},
|
||||
{"failure without a deadline", broken, false, 0, true},
|
||||
{"recent success", healthy, false, time.Minute, false},
|
||||
}
|
||||
for _, tt := range cases {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
wait, due := syncSchedule(now, tt.ops, tt.force)
|
||||
if wait != tt.wait || due != tt.due {
|
||||
t.Fatalf("schedule = (%s, %t), want (%s, %t)", wait, due, tt.wait, tt.due)
|
||||
}
|
||||
})
|
||||
}
|
||||
if backoff := syncBackoff(now, waiting); backoff != 3*time.Hour+time.Minute {
|
||||
t.Fatalf("rate-limited backoff = %s, want the bank's own deadline", backoff)
|
||||
}
|
||||
if backoff := syncBackoff(now, broken); backoff != time.Hour {
|
||||
t.Fatalf("failure backoff = %s, want hourly retries", backoff)
|
||||
}
|
||||
if backoff := syncBackoff(now, healthy); backoff != 24*time.Hour {
|
||||
t.Fatalf("successful backoff = %s, want daily synchronization", backoff)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user