Skip to content

Commit 989b9db

Browse files
authored
fix: recover transient sync timeouts (#76)
Retry transient Walmart and Monarch timeouts, safely resume interrupted multi-delivery consolidation, correct misleading success summaries, and update golang.org/x/text to fix GO-2026-5970.
1 parent 8cb5ee9 commit 989b9db

11 files changed

Lines changed: 355 additions & 33 deletions

File tree

docs/bug-fixes.md

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,66 @@ Each bug fix entry should include:
1212

1313
## Bug Fixes
1414

15+
### 2026-07-22: Reachable infinite-loop vulnerability in `golang.org/x/text`
16+
17+
**Description:**
18+
The `govulncheck` gate on PR #76 reported GO-2026-5970, an infinite loop on invalid input in `golang.org/x/text`. Itemize resolved `v0.37.0`, and the scanner found a reachable path through the CLI's Sentry-backed telemetry dependencies.
19+
20+
**Test Case:**
21+
```bash
22+
go run golang.org/x/vuln/cmd/govulncheck@v1.3.0 ./...
23+
# Before: reports reachable GO-2026-5970 and exits nonzero.
24+
# After: reports zero reachable vulnerabilities.
25+
```
26+
27+
**Root Cause:**
28+
The module graph selected `golang.org/x/text v0.37.0`, which predates the upstream fix in `v0.39.0`.
29+
30+
**Fix Applied:**
31+
Upgraded the indirect `golang.org/x/text` dependency to `v0.39.0`.
32+
33+
**Verification:**
34+
- The same `govulncheck` command that failed in CI passes locally.
35+
- Full pre-commit and race suites pass.
36+
37+
---
38+
39+
### 2026-07-22: Transient Walmart and Monarch timeouts caused avoidable order failures
40+
41+
**Description:**
42+
A production Walmart sync hit one 30-second Monarch update timeout and one 30-second Walmart ledger timeout. The Monarch multi-delivery consolidation stopped after its first idempotent update attempt. The Walmart handler then triggered a second ledger request while checking refunds; that request succeeded, but the original charge lookup had already returned an error, so the order fell back to its summary total and failed to match. The CLI nevertheless ended with `Sync completed successfully.` while reporting two errors.
43+
44+
**Test Case:**
45+
```go
46+
// internal/adapters/providers/walmart/order_multi_delivery_test.go:
47+
// TestOrder_GetFinalCharges/retries_a_transient_ledger_timeout
48+
49+
// internal/application/sync/consolidator_test.go:
50+
// TestConsolidator_ConsolidateTransactions/retries_a_transient_primary_update_timeout
51+
// TestConsolidator_ConsolidateTransactions/resumed_consolidation_preserves_the_original_charge_note
52+
53+
// internal/application/sync/handlers/walmart_test.go:
54+
// TestWalmartHandler_ProcessOrder_MultiDelivery_ResumesInterruptedConsolidation
55+
56+
// internal/cli/output_test.go:
57+
// TestPrintSyncSummaryDoesNotReportSuccessWhenOrdersFailed
58+
```
59+
60+
**Root Cause:**
61+
The Walmart adapter relied on the upstream client's retry policy, which retries rate limits but not HTTP client timeouts. Consolidation made only one Monarch update attempt even though it sets the same amount and note on every attempt. Summary wording depended only on whether at least one order was processed, not whether any other order failed.
62+
63+
**Fix Applied:**
64+
Walmart ledger reads and the idempotent Monarch consolidation update now retry once for network timeouts, while respecting caller cancellation and returning permanent errors immediately. Every Monarch update attempt remains in the API audit log. If an update reached Monarch but its response timed out, a later run recognizes the exact original consolidation note, finds any still-posted component transactions, preserves the note, and finishes deleting those extras before applying splits. Production summaries now say `Sync completed with N errors.` whenever the result contains errors and reserve the success message for error-free runs.
65+
66+
**Verification:**
67+
- All five regression tests failed before the fix and pass after it.
68+
- `go test ./...` passes.
69+
- A rebuilt live dry-run recognized the interrupted `$70.28` consolidation and identified its still-posted `$64.22` component transaction.
70+
- The authorized production repair preserved the `$70.28` primary, deleted its `$64.22` leftover, and applied two splits. It also consolidated the `$75.66` order from `$6.16 + $9.51 + $58.99`, deleted both extras, and applied two splits.
71+
- Direct Monarch read-back returned both primary amounts, notes, and two splits each; all three deleted component IDs returned `ErrNotFound`.
72+
73+
---
74+
1575
### 2026-07-16: Completed Walmart in-store receipts were treated as payment pending
1676

1777
**Description:**

go.mod

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ require (
3737
golang.org/x/net v0.55.0 // indirect
3838
golang.org/x/sync v0.21.0 // indirect
3939
golang.org/x/sys v0.45.0 // indirect
40-
golang.org/x/text v0.37.0 // indirect
40+
golang.org/x/text v0.39.0 // indirect
4141
modernc.org/libc v1.73.4 // indirect
4242
modernc.org/mathutil v1.7.1 // indirect
4343
modernc.org/memory v1.11.0 // indirect

go.sum

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -76,8 +76,8 @@ go.uber.org/multierr v1.11.0/go.mod h1:20+QtiLqy0Nd6FdQB9TLXag12DsQkrbs3htMFfDN8
7676
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
7777
golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
7878
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
79-
golang.org/x/mod v0.36.0 h1:JJjpVx6myfUsUdAzZuOSTTmRE0PfZeNWzzvKrP7amb4=
80-
golang.org/x/mod v0.36.0/go.mod h1:moc6ELqsWcOw5Ef3xVprK5ul/MvtVvkIXLziUOICjUQ=
79+
golang.org/x/mod v0.37.0 h1:vF1DjpVEshcIqoEaauuHebaLk1O1forxjxBaVn884JQ=
80+
golang.org/x/mod v0.37.0/go.mod h1:m8S8VeM9r4dzDwjrKO0a1sZP3YjeMamRRlD+fmR2Q/0=
8181
golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
8282
golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
8383
golang.org/x/net v0.0.0-20210916014120-12bc252f5db8/go.mod h1:9nx3DQGgdP8bBQD5qxJ1jj9UTztislL4KSBs9R2vV5Y=
@@ -106,13 +106,13 @@ golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
106106
golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
107107
golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
108108
golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
109-
golang.org/x/text v0.37.0 h1:Cqjiwd9eSg8e0QAkyCaQTNHFIIzWtidPahFWR83rTrc=
110-
golang.org/x/text v0.37.0/go.mod h1:a5sjxXGs9hsn/AJVwuElvCAo9v8QYLzvavO5z2PiM38=
109+
golang.org/x/text v0.39.0 h1:UbZz4pLOvn600D6Oh6GGEI6VAmndrEBLv8/6BEXzyus=
110+
golang.org/x/text v0.39.0/go.mod h1:3UwRclnC2g0TU9x8PZiyfOajCd1zaUNHF9cvqcQZ+ZM=
111111
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
112112
golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
113113
golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc=
114-
golang.org/x/tools v0.45.0 h1:18qN3FAooORvApf5XjCXgsuayZOEtXf6JK18I3+ONa8=
115-
golang.org/x/tools v0.45.0/go.mod h1:LuUGqqaXcXMEFEruIVJVm5mgDD8vww/z/SR1gQ4uE/0=
114+
golang.org/x/tools v0.47.0 h1:7Kn5x/d1svx/PzryTsqeoZN4TZwqeH5pGWjefhLi/1Q=
115+
golang.org/x/tools v0.47.0/go.mod h1:dFHnyTvFWY212G+h7ZY4Vsp/K3U4/7W9TyVaAul8uCA=
116116
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
117117
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
118118
gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk=

internal/adapters/providers/walmart/order.go

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,10 @@ package walmart
22

33
import (
44
"context"
5+
"errors"
56
"fmt"
67
"log/slog"
8+
"net"
79
"time"
810

911
"github.com/eshaffer321/itemize/internal/adapters/providers"
@@ -314,13 +316,37 @@ func (o *Order) getLedger() (*walmartclient.OrderLedger, error) {
314316
ctx = context.Background()
315317
}
316318

317-
ledger, err := o.client.GetOrderLedger(ctx, o.GetID())
318-
if err != nil {
319-
return nil, fmt.Errorf("failed to get order ledger: %w", err)
319+
const maxAttempts = 2
320+
var lastErr error
321+
for attempt := 1; attempt <= maxAttempts; attempt++ {
322+
ledger, err := o.client.GetOrderLedger(ctx, o.GetID())
323+
if err == nil {
324+
o.ledgerCache = ledger
325+
return ledger, nil
326+
}
327+
328+
lastErr = err
329+
if attempt == maxAttempts || !isRetryableLedgerError(ctx, err) {
330+
break
331+
}
332+
if o.logger != nil {
333+
o.logger.Warn("transient ledger request failed; retrying",
334+
"order_id", o.GetID(),
335+
"attempt", attempt,
336+
"error", err)
337+
}
338+
}
339+
340+
return nil, fmt.Errorf("failed to get order ledger: %w", lastErr)
341+
}
342+
343+
func isRetryableLedgerError(ctx context.Context, err error) bool {
344+
if ctx.Err() != nil || errors.Is(err, context.Canceled) {
345+
return false
320346
}
321347

322-
o.ledgerCache = ledger
323-
return ledger, nil
348+
var networkErr net.Error
349+
return errors.As(err, &networkErr) && networkErr.Timeout()
324350
}
325351

326352
// IsMultiDelivery checks if order was split into multiple deliveries

internal/adapters/providers/walmart/order_multi_delivery_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,30 @@
11
package walmart
22

33
import (
4+
"context"
45
"io"
56
"log/slog"
7+
"net/url"
68
"testing"
79

810
walmartclient "github.com/eshaffer321/walmart-client-go/v2"
911
"github.com/stretchr/testify/assert"
1012
"github.com/stretchr/testify/require"
1113
)
1214

15+
type flakyLedgerClient struct {
16+
calls int
17+
ledger *walmartclient.OrderLedger
18+
}
19+
20+
func (c *flakyLedgerClient) GetOrderLedger(_ context.Context, _ string) (*walmartclient.OrderLedger, error) {
21+
c.calls++
22+
if c.calls == 1 {
23+
return nil, &url.Error{Op: "Get", URL: "https://www.walmart.com/order-ledger", Err: context.DeadlineExceeded}
24+
}
25+
return c.ledger, nil
26+
}
27+
1328
func TestOrder_GetRefundItems_UsesClientReturnIDMetadata(t *testing.T) {
1429
order := &Order{walmartOrder: &walmartclient.Order{
1530
ID: "REFUND-ITEMS",
@@ -35,6 +50,26 @@ func TestOrder_GetRefundItems_UsesClientReturnIDMetadata(t *testing.T) {
3550

3651
// TestOrder_GetFinalCharges tests retrieving final charges from order ledger
3752
func TestOrder_GetFinalCharges(t *testing.T) {
53+
t.Run("retries a transient ledger timeout", func(t *testing.T) {
54+
client := &flakyLedgerClient{ledger: &walmartclient.OrderLedger{
55+
OrderID: "RETRY-LEDGER",
56+
PaymentMethods: []walmartclient.PaymentMethodCharges{{
57+
PaymentType: "CREDITCARD",
58+
FinalCharges: []float64{6.06, 64.22},
59+
}},
60+
}}
61+
order := &Order{
62+
walmartOrder: &walmartclient.Order{ID: "RETRY-LEDGER"},
63+
client: client,
64+
}
65+
66+
charges, err := order.GetFinalCharges()
67+
68+
require.NoError(t, err)
69+
assert.Equal(t, []float64{6.06, 64.22}, charges)
70+
assert.Equal(t, 2, client.calls)
71+
})
72+
3873
t.Run("single delivery order", func(t *testing.T) {
3974
order := &Order{
4075
walmartOrder: &walmartclient.Order{ID: "TEST123"},

internal/application/sync/consolidator.go

Lines changed: 40 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,17 @@ package sync
33
import (
44
"context"
55
"encoding/json"
6+
"errors"
67
"fmt"
78
"log/slog"
89
"math"
10+
"net"
911
"strings"
1012
"time"
1113

12-
"github.com/eshaffer321/monarch-go/v2/pkg/monarch"
1314
"github.com/eshaffer321/itemize/internal/adapters/providers"
1415
"github.com/eshaffer321/itemize/internal/infrastructure/storage"
16+
"github.com/eshaffer321/monarch-go/v2/pkg/monarch"
1517
)
1618

1719
// Consolidator handles transaction consolidation for multi-delivery orders
@@ -159,6 +161,12 @@ func (c *Consolidator) updatePrimaryTransaction(
159161
) (*monarch.Transaction, error) {
160162
// Build consolidation note
161163
note := c.buildConsolidationNote(allTransactions)
164+
if math.Abs(math.Abs(primary.Amount)-math.Abs(order.GetTotal())) <= 0.01 &&
165+
strings.HasPrefix(primary.Notes, "Multi-delivery order (") {
166+
// A prior update may have reached Monarch even if its HTTP response timed
167+
// out. Preserve the original charge audit note while deleting leftovers.
168+
note = primary.Notes
169+
}
162170

163171
// Calculate new amount (match sign of original transaction)
164172
newAmount := order.GetTotal()
@@ -193,15 +201,27 @@ func (c *Consolidator) updatePrimaryTransaction(
193201
Notes: &note,
194202
}
195203

196-
start := time.Now()
197-
updated, err := c.client.Transactions.Update(ctx, primary.ID, params)
198-
duration := time.Since(start).Milliseconds()
204+
const maxAttempts = 2
205+
var updated *monarch.Transaction
206+
var err error
207+
for attempt := 1; attempt <= maxAttempts; attempt++ {
208+
start := time.Now()
209+
updated, err = c.client.Transactions.Update(ctx, primary.ID, params)
210+
duration := time.Since(start).Milliseconds()
199211

200-
// Log API call
201-
c.logAPICall(order.GetID(), "Transactions.Update", params, updated, err, duration)
212+
// Log every attempt so timeout recovery remains auditable.
213+
c.logAPICall(order.GetID(), "Transactions.Update", params, updated, err, duration)
202214

203-
if err != nil {
204-
return nil, fmt.Errorf("failed to update transaction %s: %w", primary.ID, err)
215+
if err == nil {
216+
break
217+
}
218+
if attempt == maxAttempts || !isRetryableMonarchError(ctx, err) {
219+
return nil, fmt.Errorf("failed to update transaction %s: %w", primary.ID, err)
220+
}
221+
c.logger.Warn("Transient transaction update failed; retrying",
222+
"transaction_id", primary.ID,
223+
"attempt", attempt,
224+
"error", err)
205225
}
206226

207227
c.logger.Info("Updated primary transaction",
@@ -212,6 +232,18 @@ func (c *Consolidator) updatePrimaryTransaction(
212232
return updated, nil
213233
}
214234

235+
func isRetryableMonarchError(ctx context.Context, err error) bool {
236+
if ctx.Err() != nil || errors.Is(err, context.Canceled) {
237+
return false
238+
}
239+
if monarch.IsRetryable(err) {
240+
return true
241+
}
242+
243+
var networkErr net.Error
244+
return errors.As(err, &networkErr) && networkErr.Timeout()
245+
}
246+
215247
// deleteExtraTransactions removes the extra transactions after consolidation
216248
// Returns list of transaction IDs that failed to delete
217249
func (c *Consolidator) deleteExtraTransactions(

0 commit comments

Comments
 (0)