RomanNum3ral
/
whisper-money
mirror of https://github.com/whisper-money/whisper-money.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix(wise): send the pagination cursor under the name Wise reads (#788) 

> Sentry's MCP token is still expired, so this came from the production
DB and `failed_jobs` again — the follow-up I flagged in #782.

## The bug

A user connected Wise on 2026-07-29 and **has never received a completed
sync in 14 days**. Their wallet holds 110 transactions but **zero rows
in `account_balances`**, so it contributes nothing to their net worth,
and the connection shows a red Error badge with no notification ever
sent.

## Root cause: one word

`WiseClient::getActivities` sent the pagination cursor as `cursor`. Wise
*returns* it as `cursor` but only *reads* it as `nextCursor` — the docs
say it outright ("Pass this value as the `nextCursor` query parameter").
We sent the wrong name, Wise ignored it, and **every request returned
page one again**. The walk could never terminate.

The production data says the same thing without the docs:

| created_at | rows | transaction_date range |
|---|---|---|
| 2026-07-29 06:44:49 | 87 | 2025-12-23 → 2026-07-20 |
| 2026-07-29 06:44:50 | 10 | 2025-12-19 → 2025-12-23 |
| every run since | 1/day | that day only |

97 rows in two consecutive seconds — one `size=100` page after
`CARD_CHECK`/non-EUR filtering — and in ~50 runs over 14 days **never a
row older than 2025-12-19**. Page two has never been fetched.

The timeouts were a symptom, not the cause: the loop hammered one
endpoint for 120s straight, four times a day, until Wise started
answering with cURL-28s and a 500. 65 of the 66
`TimeoutExceededException` job failures in `failed_jobs` over 14 days
are this one connection, which also burns three 120s attempts plus three
worker kills per cycle on the single `default` worker, delaying everyone
else's jobs.

**I had this wrong.** My first pass diagnosed "a year of history is too
much to paginate" and capped the walk at 60s. That would have converted
an infinite loop into a permanent 100-activity ceiling on every Wise
account — masking the bug while looking like a fix. The product review
caught it; I verified it against both the Wise docs and the import
timestamps before rewriting.

## The commits

1. **`nextCursor`.** The root cause. The test keys its fake off
`nextCursor`, so the old name looks like what it was — a one-page
history that never ends. With the wrong name the test does not terminate
(verified under an alarm); with the right one it pages twice and stops.
2. **A time budget, as a safety net rather than the fix.** With
pagination working a normal wallet finishes in two requests, but a long
history or a slow provider would still get the job killed, and
`last_synced_at` is only written on success — which is exactly the
never-converges state. The deadline is the *caller's*, passed in: Wise
creates one account per currency per profile, so a budget per wallet
multiplies straight past the job's 120s (a three-wallet connection
reproduced the original bug verbatim — there is a test). Null lets
`banking:sync --sync`, which runs in-process without the worker timeout,
walk as far as it likes. `WiseClient` now owns 15s/5s timeouts instead
of inheriting the framework's 30s, so "budget plus one in-flight
request" is a bound this code can actually state. Matches the two
sibling clients.
3. **Balance first, and not load-bearing.** The balance ran after the
walk, which never returned — hence zero balance rows. Ordering it first
is only half the fix: `getBorderlessAccount` throws on a 5xx and nothing
caught it, so done naively it just swaps which half the user loses. It
is wrapped, counted into the returned metadata like
`EnableBankingSyncer` does, and skipped wallets are reported too.

## Verification

`tests/Feature/OpenBanking`: 354 tests, 344 pass, and the **same 10
failures as clean main** (Inertia page-render tests hitting the SSR
`/render` endpoint with no local server — baseline confirmed). 4 new
tests, each verified to fail with only its own change reverted:
per-wallet budget → the multi-wallet test; no try/catch → the balance
test; wrong cursor name → non-termination. `pint`, `crap` (0 methods
over 10 — `importPage` is extracted because the deadline pushed `sync`
to 11) and `dry` all green.

## Not done, deliberately

- **Wise has no historical-balance backfill**, unlike
Coinbase/IBKR/EnableBanking — `WiseBalanceSyncService` only ever writes
*today's* balance, and `BalanceLookup::getBalanceAt` returns 0 with no
earlier row. So this wallet will read €0 across the whole 12-month
sparkline and step to its real value the day this ships, next to 110
transactions going back to December. Pre-existing and true of any new
Wise connection, but this fix is what makes it visible. Its own PR.
- **N wallets on one profile each walk the identical list** — the
activities endpoint is per profile, and `parseActivity` filters by
currency afterwards. Fetch once per profile and fan out; real cost and
rate-limit win, bigger change.
- **Backfilling older history.** The next sync starts from the
connection-level `last_synced_at`, so pages left behind on a budget stop
are not revisited. `EnableBankingSyncer::resolveDateFrom` already has
the cheap pattern (derive the window from the imported rows, no schema
change) — for Wise's newest→oldest walk that means setting `until` to
the oldest imported row. Noted as the upgrade path in the code rather
than "persist a cursor", which needs a migration.
- **14 days broken and silent.** No notification exists for a connection
stuck in Error or never-synced, and since #757 correctly stopped
counting transient failures there is no escalation either. Worth an
alert on days-since-last-success; called out as a follow-up in #782 too.

## Auto-merge

Enabled. The root cause is a one-word parameter name confirmed against
the vendor docs and independently against production data; the other two
commits are additive safety with tests that each fail without them. No
migration, no data writes, no schema change, and the affected code path
serves one production connection that is currently completely broken —
the downside of being wrong is bounded by that, and the upside is a user
who gets their account back.
This commit is contained in:
Víctor Falcón 2026-08-12 13:30:37 +02:00 committed by GitHub
parent 2f00a56272
commit 2c7fe5d64a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 308 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

57
app/Services/Banking/Sync/WiseSyncer.php
View File

@ -2,13 +2,32 @@
namespace App\Services\Banking\Sync;
use App\Exceptions\Banking\TransientBankingProviderException;
use App\Models\BankingConnection;
use App\Services\Banking\WiseBalanceSyncService;
use App\Services\Banking\WiseClient;
use App\Services\Banking\WiseTransactionSyncService;
use Illuminate\Support\Facades\Log;
class WiseSyncer extends AbstractBankingConnectionSyncer
{
/**
* Wall-clock budget for the whole connection, sized against
* SyncBankingConnectionJob's 120s timeout.
*
* A safety net rather than a policy: with the cursor fixed a normal wallet
* finishes in a couple of requests. It exists so that a genuinely long history
* - or a provider answering slowly - stops the run short instead of getting the
* job killed, which is what used to leave last_synced_at unset and every cycle
* restarting the same first sync.
*
* Per connection, not per wallet: Wise creates one account per currency per
* profile, so a budget each would multiply straight past the job's timeout.
* Worst case is this budget plus one in-flight request (WiseClient caps those),
* which leaves headroom inside 120s.
*/
private const int SYNC_BUDGET_SECONDS = 90;
public function __construct(
private WiseTransactionSyncService $transactionSync,
private WiseBalanceSyncService $balanceSync,
@ -22,20 +41,52 @@ class WiseSyncer extends AbstractBankingConnectionSyncer
$dateTo = now()->toDateString();
$client = new WiseClient($connection->api_token);
$deadline = now()->addSeconds(self::SYNC_BUDGET_SECONDS);
$connection->load('accounts');
$transactionsPerAccount = [];
$balancesFailed = 0;
$walletsSkipped = 0;
foreach ($connection->accounts as $account) {
$count = $this->transactionSync->sync($account, $client, $dateFrom, $dateTo);
$this->balanceSync->sync($account, $client);
$transactionsPerAccount[$account->name] = $count;
if (now()->gte($deadline)) {
// Better to leave a wallet for the next cycle than to have the job
// killed, which would discard the sync timestamp for all of them.
$walletsSkipped++;
continue;
}
// Balance first, and its failure must not cost the transactions: it is
// one request against a walk that can span many, so ordering it second
// meant a wallet whose history could not be walked was missing from net
// worth entirely.
try {
$this->balanceSync->sync($account, $client);
} catch (TransientBankingProviderException $e) {
$balancesFailed++;
Log::warning('Wise balance sync failed, continuing', [
'account_id' => $account->id,
'error' => $e->getMessage(),
]);
}
$transactionsPerAccount[$account->name] = $this->transactionSync->sync(
$account,
$client,
$dateFrom,
$dateTo,
$deadline,
);
}
return [
'transactions_synced' => array_sum($transactionsPerAccount),
'transactions_per_account' => $transactionsPerAccount,
'balances_failed' => $balancesFailed,
'wallets_skipped_on_budget' => $walletsSkipped,
];
}
}

17
app/Services/Banking/WiseClient.php
View File

@ -13,6 +13,15 @@ class WiseClient
{
private const BASE_URL = 'https://api.wise.com';
/**
* Explicit rather than the framework's 30s default: the caller's time budget is
* stated as "the budget plus one in-flight request", which only holds if this
* class owns that number. Matches the sibling banking clients.
*/
private const int HTTP_TIMEOUT_SECONDS = 15;
private const int HTTP_CONNECT_TIMEOUT_SECONDS = 5;
public function __construct(private string $apiToken) {}
/**
@ -39,6 +48,10 @@ class WiseClient
* Fetch paginated monetary activities for a profile.
* Use `since`/`until` (ISO 8601) for date range and `cursor` for pagination.
*
* The names are asymmetric and it matters: Wise returns the cursor as
* `cursor` but only reads it back as `nextCursor`. Sending it as `cursor` is
* silently ignored, so every request returns the first page again.
*
* @return array{activities?: array, cursor?: string|null}
*/
public function getActivities(int $profileId, string $since, string $until, ?string $cursor = null): array
@ -50,7 +63,7 @@ class WiseClient
];
if ($cursor !== null) {
$params['cursor'] = $cursor;
$params['nextCursor'] = $cursor;
}
return $this->get("/v1/profiles/{$profileId}/activities", $params);
@ -97,6 +110,8 @@ class WiseClient
private function client(): PendingRequest
{
return Http::baseUrl(self::BASE_URL)
->timeout(self::HTTP_TIMEOUT_SECONDS)
->connectTimeout(self::HTTP_CONNECT_TIMEOUT_SECONDS)
->withToken($this->apiToken)
->acceptJson()
->throw(function ($response, RequestException $exception) {

68
app/Services/Banking/WiseTransactionSyncService.php
View File

@ -4,6 +4,7 @@ namespace App\Services\Banking;
use App\Enums\TransactionSource;
use App\Models\Account;
use Carbon\CarbonInterface;
use Illuminate\Database\UniqueConstraintViolationException;
use Illuminate\Support\Facades\Log;
@ -15,9 +16,14 @@ class WiseTransactionSyncService
* The account's `external_account_id` must be in the format
* "{profileId}:{currency}" (e.g. "36875276:EUR").
*
* @param CarbonInterface|null $deadline Wall clock at which to stop paging and
* leave the rest for the next sync. The
* caller owns it because the budget is
* per connection, not per wallet. Null
* for callers under no time pressure.
* @return int Number of new transactions created
*/
public function sync(Account $account, WiseClient $client, string $dateFrom, string $dateTo): int
public function sync(Account $account, WiseClient $client, string $dateFrom, string $dateTo, ?CarbonInterface $deadline = null): int
{
if (! $account->external_account_id) {
return 0;
@ -29,29 +35,18 @@ class WiseTransactionSyncService
$until = $dateTo.'T23:59:59Z';
$cursor = null;
$created = 0;
$ranOutOfBudget = false;
do {
$result = $client->getActivities((int) $profileId, $since, $until, $cursor);
$activities = $result['activities'] ?? [];
$cursor = $result['cursor'] ?? null;
foreach ($activities as $activity) {
// Skip zero-amount authorization checks and non-monetary types
if (($activity['type'] ?? '') === 'CARD_CHECK') {
continue;
}
$created += $this->importPage($account, $activities, $currency);
$parsed = $this->parseActivity($activity, $currency);
if ($parsed === null) {
continue;
}
if ($this->importTransaction($account, $activity, $parsed)) {
$created++;
}
}
} while ($cursor !== null && count($activities) > 0);
$hasMorePages = $cursor !== null && count($activities) > 0;
$ranOutOfBudget = $hasMorePages && $deadline !== null && now()->gte($deadline);
} while ($hasMorePages && ! $ranOutOfBudget);
Log::info('Synced Wise transactions', [
'account_id' => $account->id,
@ -61,6 +56,45 @@ class WiseTransactionSyncService
'date_to' => $dateTo,
]);
if ($ranOutOfBudget) {
// Warn, not info: pages were left behind and the next sync starts from
// last_synced_at, so this is history the user silently does not get.
Log::warning('Wise transaction walk stopped on its time budget', [
'account_id' => $account->id,
'currency' => $currency,
'new_transactions' => $created,
'oldest_imported' => $account->transactions()->min('transaction_date'),
]);
}
return $created;
}
/**
* @param array<int, array<string, mixed>> $activities
* @return int Number of new transactions created from this page
*/
private function importPage(Account $account, array $activities, string $currency): int
{
$created = 0;
foreach ($activities as $activity) {
// Skip zero-amount authorization checks and non-monetary types
if (($activity['type'] ?? '') === 'CARD_CHECK') {
continue;
}
$parsed = $this->parseActivity($activity, $currency);
if ($parsed === null) {
continue;
}
if ($this->importTransaction($account, $activity, $parsed)) {
$created++;
}
}
return $created;
}

64
tests/Feature/OpenBanking/WisePaginationTest.php Normal file
View File

@ -0,0 +1,64 @@
<?php
use App\Models\Account;
use App\Models\BankingConnection;
use App\Models\User;
use App\Services\Banking\WiseClient;
use App\Services\Banking\WiseTransactionSyncService;
use Illuminate\Support\Facades\Http;
test('the walk pages through the whole history instead of re-reading page one', function () {
$user = User::factory()->onboarded()->create(['currency_code' => 'EUR']);
$connection = BankingConnection::factory()->wise()->create(['user_id' => $user->id]);
$account = Account::factory()->connected()->create([
'user_id' => $user->id,
'banking_connection_id' => $connection->id,
'external_account_id' => '35242290:EUR',
'currency_code' => 'EUR',
]);
// Wise hands the cursor back as `cursor` but only reads it as `nextCursor`,
// so a fake that keys off `nextCursor` is what tells the two apart: sending
// the wrong name looks exactly like a one-page history that never ends.
Http::fake([
'api.wise.com/v1/profiles/*/activities*' => function ($request) {
$cursor = $request->data()['nextCursor'] ?? null;
return Http::response(match ($cursor) {
null => [
'activities' => [activityFixture(1)],
'cursor' => 'page-2',
],
'page-2' => [
'activities' => [activityFixture(2)],
'cursor' => null,
],
default => ['activities' => [], 'cursor' => null],
});
},
]);
$created = app(WiseTransactionSyncService::class)->sync(
$account,
new WiseClient('test-token'),
now()->subYear()->toDateString(),
now()->toDateString(),
);
expect($created)->toBe(2);
Http::assertSentCount(2);
expect($account->transactions()->pluck('description')->sort()->values()->all())
->toBe(['Purchase 1', 'Purchase 2']);
});
function activityFixture(int $n): array
{
return [
'id' => "activity-{$n}",
'type' => 'CARD_PAYMENT',
'title' => "Purchase {$n}",
'primaryAmount' => '<negative>10.00 EUR</negative>',
'secondaryAmount' => '',
'createdOn' => now()->subDays($n)->toIso8601String(),
];
}

123
tests/Feature/OpenBanking/WiseSyncBudgetTest.php Normal file
View File

@ -0,0 +1,123 @@
<?php
use App\Models\Account;
use App\Models\BankingConnection;
use App\Models\User;
use App\Services\Banking\Sync\WiseSyncer;
use Illuminate\Support\Carbon;
use Illuminate\Support\Facades\Http;
/**
* A wallet with more history than one run can walk. The page count is capped so a
* broken budget fails on the request count instead of hanging CI, and each page
* advances the clock to stand in for the seconds a real request spends.
*/
function fakeLongWiseHistory(int $secondsPerPage = 20, int $pages = 40): void
{
Carbon::setTestNow(Carbon::now());
Http::fake([
'api.wise.com/v2/borderless-accounts*' => Http::response([
[
'id' => 44333087,
'profileId' => 35242290,
'balances' => [
['currency' => 'EUR', 'amount' => ['value' => 19.81, 'currency' => 'EUR']],
],
],
]),
'api.wise.com/v1/profiles/*/activities*' => function ($request) use ($secondsPerPage, $pages) {
$page = (int) str_replace('page-', '', $request->data()['nextCursor'] ?? 'page-1');
Carbon::setTestNow(Carbon::now()->addSeconds($secondsPerPage));
return Http::response([
'activities' => [[
'id' => "activity-{$page}",
'type' => 'CARD_PAYMENT',
'title' => "Purchase {$page}",
'primaryAmount' => '<negative>10.00 EUR</negative>',
'secondaryAmount' => '',
'createdOn' => now()->subDays($page)->toIso8601String(),
]],
'cursor' => $page < $pages ? 'page-'.($page + 1) : null,
]);
},
]);
}
function wiseWalletsFor(User $user, BankingConnection $connection, string ...$currencies): void
{
foreach ($currencies as $currency) {
Account::factory()->connected()->create([
'user_id' => $user->id,
'banking_connection_id' => $connection->id,
'external_account_id' => '35242290:'.$currency,
'currency_code' => $currency,
]);
}
}
afterEach(function () {
Carbon::setTestNow();
});
test('a history longer than the budget stops short instead of getting the job killed', function () {
$user = User::factory()->onboarded()->create(['currency_code' => 'EUR']);
$connection = BankingConnection::factory()->wise()->create(['user_id' => $user->id]);
wiseWalletsFor($user, $connection, 'EUR');
fakeLongWiseHistory();
$metadata = app(WiseSyncer::class)->sync($connection, isFirstSync: true);
// 20s of clock per page against a 90s budget: the fifth page closes it, so the
// walk stops well before the 40 pages on offer.
expect($metadata['transactions_synced'])->toBe(5);
expect($metadata['wallets_skipped_on_budget'])->toBe(0);
});
test('the budget covers the whole connection, not each wallet separately', function () {
$user = User::factory()->onboarded()->create(['currency_code' => 'EUR']);
$connection = BankingConnection::factory()->wise()->create(['user_id' => $user->id]);
// Wise creates one account per currency per profile, so multi-wallet is the
// normal case. A budget each would multiply past the job's 120s timeout and
// reproduce the original bug.
wiseWalletsFor($user, $connection, 'EUR', 'USD', 'GBP');
fakeLongWiseHistory();
$metadata = app(WiseSyncer::class)->sync($connection, isFirstSync: true);
expect($metadata['wallets_skipped_on_budget'])->toBeGreaterThan(0);
// The whole run stays within one budget's worth of pages rather than one per
// wallet, which is what keeps it inside the job's timeout.
expect(count(Http::recorded()))->toBeLessThan(12);
});
test('a wallet whose balance request fails still gets its transactions', function () {
$user = User::factory()->onboarded()->create(['currency_code' => 'EUR']);
$connection = BankingConnection::factory()->wise()->create(['user_id' => $user->id]);
wiseWalletsFor($user, $connection, 'EUR');
Http::fake([
'api.wise.com/v2/borderless-accounts*' => Http::response(['message' => 'boom'], 500),
'api.wise.com/v1/profiles/*/activities*' => Http::response([
'activities' => [[
'id' => 'activity-1',
'type' => 'CARD_PAYMENT',
'title' => 'Purchase 1',
'primaryAmount' => '<negative>10.00 EUR</negative>',
'secondaryAmount' => '',
'createdOn' => now()->subDay()->toIso8601String(),
]],
'cursor' => null,
]),
]);
$metadata = app(WiseSyncer::class)->sync($connection, isFirstSync: true);
// Ordering the balance first must not make it load-bearing: before this, a
// balance hiccup would have cost the user their transactions too.
expect($metadata['balances_failed'])->toBe(1);
expect($metadata['transactions_synced'])->toBe(1);
});