fix(banking): handle balance-fetch timeouts and silence handled retries (#450)
Fixes two linked production banking-sync issues. ## PHP-LARAVEL-W — `cURL error 28: timed out` in `EnableBankingProvider::getBalances` (8 events, 4 users, High) `getBalances` called `$response->throw()` raw, so a connection timeout (or ASPSP error) escaped as an **unhandled** `ConnectionException`/`RequestException` and crashed the sync. `getTransactions` already wraps these in `TransientBankingProviderException` (which `implements ShouldntReport` and is handled as a transient, retryable error in `SyncBankingConnectionJob`). → `getBalances` now follows the exact same pattern. Genuine validation errors (non-ASPSP 4xx) stay reportable. ## PHP-LARAVEL-2D — `SyncBankingConnectionJob has been attempted too many times` (High, regressed) The hanging balance call above pushed the job past its 120s `timeout`, the worker was killed mid-job, and the retry tripped a `MaxAttemptsExceededException`. That exception is thrown by the queue worker (not catchable in `handle()`), and the job's `failed()` handler **already** records the terminal `Error` state on the connection — so the Sentry report is redundant operational noise. → Fixing W removes the main cause of the timeout. Additionally, `MaxAttemptsExceededException` is no longer reported **for this job only** (scoped via `dontReportWhen` on `$e->job?->resolveName()`); other jobs still report it. ## Tests - `getBalances` wraps connection failures and ASPSP errors as non-reportable transient errors; keeps non-ASPSP client errors reportable. - `MaxAttemptsExceededException` is not reported for `SyncBankingConnectionJob`, but still reported for other jobs. Fixes PHP-LARAVEL-W, PHP-LARAVEL-2D.
This commit is contained in:
parent
bef657c2ed
commit
64b78e3680
|
|
@ -0,0 +1,55 @@
|
||||||
|
# Welcome to Whisper Money
|
||||||
|
|
||||||
|
## How We Use Claude
|
||||||
|
|
||||||
|
Based on Víctor Falcón's usage over the last 30 days:
|
||||||
|
|
||||||
|
Work Type Breakdown:
|
||||||
|
Debug Fix ████████████████████ 100%
|
||||||
|
|
||||||
|
Top Skills & Commands:
|
||||||
|
/sentry-cli ████████████████████ 1x/month
|
||||||
|
|
||||||
|
Top MCP Servers:
|
||||||
|
_None used in this window_
|
||||||
|
|
||||||
|
## Your Setup Checklist
|
||||||
|
|
||||||
|
### Codebases
|
||||||
|
- [ ] whisper-money — https://github.com/whisper-money/whisper-money
|
||||||
|
|
||||||
|
### MCP Servers to Activate
|
||||||
|
- [ ] _None required yet_ — the team hasn't leaned on MCP servers in this window. Sentry access is via the `/sentry-cli` skill (see below).
|
||||||
|
|
||||||
|
### Skills to Know About
|
||||||
|
- [ ] `/sentry-cli` — drives Sentry from the command line (view issues, events, projects, orgs, make API calls). The team uses it when triaging and fixing flagged errors before opening a PR.
|
||||||
|
|
||||||
|
## Team Tips
|
||||||
|
|
||||||
|
_TODO_
|
||||||
|
|
||||||
|
## Get Started
|
||||||
|
|
||||||
|
_TODO_
|
||||||
|
|
||||||
|
<!-- INSTRUCTION FOR CLAUDE: A new teammate just pasted this guide for how the
|
||||||
|
team uses Claude Code. You're their onboarding buddy — warm, conversational,
|
||||||
|
not lecture-y.
|
||||||
|
|
||||||
|
Open with a warm welcome — include the team name from the title. Then: "Your
|
||||||
|
teammate uses Claude Code for [list all the work types]. Let's get you started."
|
||||||
|
|
||||||
|
Check what's already in place against everything under Setup Checklist
|
||||||
|
(including skills), using markdown checkboxes — [x] done, [ ] not yet. Lead
|
||||||
|
with what they already have. One sentence per item, all in one message.
|
||||||
|
|
||||||
|
Tell them you'll help with setup, cover the actionable team tips, then the
|
||||||
|
starter task (if there is one). Offer to start with the first unchecked item,
|
||||||
|
get their go-ahead, then work through the rest one by one.
|
||||||
|
|
||||||
|
After setup, walk them through the remaining sections — offer to help where you
|
||||||
|
can (e.g. link to channels), and just surface the purely informational bits.
|
||||||
|
|
||||||
|
Don't invent sections or summaries that aren't in the guide. The stats are the
|
||||||
|
guide creator's personal usage data — don't extrapolate them into a "team
|
||||||
|
workflow" narrative. -->
|
||||||
|
|
@ -129,9 +129,32 @@ class EnableBankingProvider implements BankingProviderInterface
|
||||||
|
|
||||||
public function getBalances(string $accountId): array
|
public function getBalances(string $accountId): array
|
||||||
{
|
{
|
||||||
$response = $this->client()->get("/accounts/{$accountId}/balances");
|
try {
|
||||||
|
$response = $this->client()->get("/accounts/{$accountId}/balances");
|
||||||
|
|
||||||
$response->throw();
|
$response->throw();
|
||||||
|
} catch (ConnectionException $e) {
|
||||||
|
throw new TransientBankingProviderException(
|
||||||
|
'EnableBanking did not respond while fetching account balances.',
|
||||||
|
provider: 'enablebanking',
|
||||||
|
previous: $e,
|
||||||
|
);
|
||||||
|
} catch (RequestException $e) {
|
||||||
|
if (! $this->isAspspError($e)) {
|
||||||
|
throw $e;
|
||||||
|
}
|
||||||
|
|
||||||
|
$body = $this->errorBody($e);
|
||||||
|
$providerCode = $body['error'] ?? null;
|
||||||
|
|
||||||
|
throw new TransientBankingProviderException(
|
||||||
|
'EnableBanking bank connector failed while fetching account balances.',
|
||||||
|
provider: 'enablebanking',
|
||||||
|
statusCode: $e->response->status(),
|
||||||
|
providerCode: is_string($providerCode) ? $providerCode : null,
|
||||||
|
previous: $e,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
return $response->json();
|
return $response->json();
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -7,12 +7,14 @@ use App\Http\Middleware\HandleAppearance;
|
||||||
use App\Http\Middleware\HandleInertiaRequests;
|
use App\Http\Middleware\HandleInertiaRequests;
|
||||||
use App\Http\Middleware\SetLocale;
|
use App\Http\Middleware\SetLocale;
|
||||||
use App\Http\Middleware\SetSentryUser;
|
use App\Http\Middleware\SetSentryUser;
|
||||||
|
use App\Jobs\SyncBankingConnectionJob;
|
||||||
use App\Services\AuthEntryPointService;
|
use App\Services\AuthEntryPointService;
|
||||||
use Illuminate\Foundation\Application;
|
use Illuminate\Foundation\Application;
|
||||||
use Illuminate\Foundation\Configuration\Exceptions;
|
use Illuminate\Foundation\Configuration\Exceptions;
|
||||||
use Illuminate\Foundation\Configuration\Middleware;
|
use Illuminate\Foundation\Configuration\Middleware;
|
||||||
use Illuminate\Http\Middleware\AddLinkHeadersForPreloadedAssets;
|
use Illuminate\Http\Middleware\AddLinkHeadersForPreloadedAssets;
|
||||||
use Illuminate\Http\Request;
|
use Illuminate\Http\Request;
|
||||||
|
use Illuminate\Queue\MaxAttemptsExceededException;
|
||||||
use Sentry\Laravel\Integration;
|
use Sentry\Laravel\Integration;
|
||||||
|
|
||||||
return Application::configure(basePath: dirname(__DIR__))
|
return Application::configure(basePath: dirname(__DIR__))
|
||||||
|
|
@ -52,4 +54,7 @@ return Application::configure(basePath: dirname(__DIR__))
|
||||||
})
|
})
|
||||||
->withExceptions(function (Exceptions $exceptions): void {
|
->withExceptions(function (Exceptions $exceptions): void {
|
||||||
Integration::handles($exceptions);
|
Integration::handles($exceptions);
|
||||||
|
|
||||||
|
$exceptions->dontReportWhen(fn (Throwable $e): bool => $e instanceof MaxAttemptsExceededException
|
||||||
|
&& $e->job?->resolveName() === SyncBankingConnectionJob::class);
|
||||||
})->create();
|
})->create();
|
||||||
|
|
|
||||||
|
|
@ -71,6 +71,69 @@ test('getTransactions keeps non-ASPSP client errors reportable', function () {
|
||||||
->toThrow(RequestException::class);
|
->toThrow(RequestException::class);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('getBalances wraps connection failures as non-reportable transient errors', function () {
|
||||||
|
Http::fake([
|
||||||
|
'api.enablebanking.com/accounts/ext-123/balances' => Http::failedConnection(),
|
||||||
|
]);
|
||||||
|
|
||||||
|
$provider = enableBankingProviderForTest();
|
||||||
|
|
||||||
|
try {
|
||||||
|
$provider->getBalances('ext-123');
|
||||||
|
} catch (TransientBankingProviderException $e) {
|
||||||
|
expect($e)->toBeInstanceOf(ShouldntReport::class)
|
||||||
|
->and($e->provider)->toBe('enablebanking')
|
||||||
|
->and($e->statusCode)->toBeNull()
|
||||||
|
->and($e->providerCode)->toBeNull()
|
||||||
|
->and($e->getPrevious())->toBeInstanceOf(ConnectionException::class);
|
||||||
|
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
test()->fail('Expected transient banking provider exception.');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('getBalances wraps EnableBanking ASPSP errors as non-reportable transient errors', function () {
|
||||||
|
Http::fake([
|
||||||
|
'api.enablebanking.com/accounts/ext-123/balances' => Http::response([
|
||||||
|
'code' => 400,
|
||||||
|
'message' => 'Error interacting with ASPSP',
|
||||||
|
'error' => 'ASPSP_ERROR',
|
||||||
|
], 400),
|
||||||
|
]);
|
||||||
|
|
||||||
|
$provider = enableBankingProviderForTest();
|
||||||
|
|
||||||
|
try {
|
||||||
|
$provider->getBalances('ext-123');
|
||||||
|
} catch (TransientBankingProviderException $e) {
|
||||||
|
expect($e)->toBeInstanceOf(ShouldntReport::class)
|
||||||
|
->and($e->provider)->toBe('enablebanking')
|
||||||
|
->and($e->statusCode)->toBe(400)
|
||||||
|
->and($e->providerCode)->toBe('ASPSP_ERROR')
|
||||||
|
->and($e->getPrevious())->toBeInstanceOf(RequestException::class);
|
||||||
|
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
test()->fail('Expected transient banking provider exception.');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('getBalances keeps non-ASPSP client errors reportable', function () {
|
||||||
|
Http::fake([
|
||||||
|
'api.enablebanking.com/accounts/ext-123/balances' => Http::response([
|
||||||
|
'code' => 400,
|
||||||
|
'message' => 'Invalid account',
|
||||||
|
'error' => 'VALIDATION_ERROR',
|
||||||
|
], 400),
|
||||||
|
]);
|
||||||
|
|
||||||
|
$provider = enableBankingProviderForTest();
|
||||||
|
|
||||||
|
expect(fn () => $provider->getBalances('ext-123'))
|
||||||
|
->toThrow(RequestException::class);
|
||||||
|
});
|
||||||
|
|
||||||
function enableBankingProviderForTest(): EnableBankingProvider
|
function enableBankingProviderForTest(): EnableBankingProvider
|
||||||
{
|
{
|
||||||
$privateKey = <<<'PEM'
|
$privateKey = <<<'PEM'
|
||||||
|
|
|
||||||
|
|
@ -18,8 +18,10 @@ use App\Services\Banking\BalanceSyncService;
|
||||||
use App\Services\Banking\TransactionSyncService;
|
use App\Services\Banking\TransactionSyncService;
|
||||||
use Carbon\Carbon;
|
use Carbon\Carbon;
|
||||||
use GuzzleHttp\Psr7\Response;
|
use GuzzleHttp\Psr7\Response;
|
||||||
|
use Illuminate\Contracts\Debug\ExceptionHandler;
|
||||||
use Illuminate\Contracts\Queue\Job;
|
use Illuminate\Contracts\Queue\Job;
|
||||||
use Illuminate\Http\Client\RequestException;
|
use Illuminate\Http\Client\RequestException;
|
||||||
|
use Illuminate\Queue\MaxAttemptsExceededException;
|
||||||
use Illuminate\Support\Facades\Http;
|
use Illuminate\Support\Facades\Http;
|
||||||
use Illuminate\Support\Facades\Mail;
|
use Illuminate\Support\Facades\Mail;
|
||||||
use Illuminate\Support\Facades\Queue;
|
use Illuminate\Support\Facades\Queue;
|
||||||
|
|
@ -1454,3 +1456,21 @@ test('successful sync clears rate_limited_until', function () {
|
||||||
$connection->refresh();
|
$connection->refresh();
|
||||||
expect($connection->rate_limited_until)->toBeNull();
|
expect($connection->rate_limited_until)->toBeNull();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('does not report MaxAttemptsExceededException raised for SyncBankingConnectionJob', function () {
|
||||||
|
$queueJob = Mockery::mock(Job::class);
|
||||||
|
$queueJob->shouldReceive('resolveName')->andReturn(SyncBankingConnectionJob::class);
|
||||||
|
|
||||||
|
$exception = MaxAttemptsExceededException::forJob($queueJob);
|
||||||
|
|
||||||
|
expect(app(ExceptionHandler::class)->shouldReport($exception))->toBeFalse();
|
||||||
|
});
|
||||||
|
|
||||||
|
test('still reports MaxAttemptsExceededException raised for other jobs', function () {
|
||||||
|
$queueJob = Mockery::mock(Job::class);
|
||||||
|
$queueJob->shouldReceive('resolveName')->andReturn(SendDailyBankTransactionsSyncedEmailJob::class);
|
||||||
|
|
||||||
|
$exception = MaxAttemptsExceededException::forJob($queueJob);
|
||||||
|
|
||||||
|
expect(app(ExceptionHandler::class)->shouldReport($exception))->toBeTrue();
|
||||||
|
});
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue