From 0989aa7c381882757baf0a7cfc0f4149024b42bb Mon Sep 17 00:00:00 2001 From: Craig Potter Date: Sat, 18 Jul 2026 09:39:57 +0100 Subject: [PATCH] Improve recording accuracy - keep_successful_responses=false now still records the status, outcome and duration of successful responses - only the body and headers are omitted, so rows no longer read as failures - Measure the max_response_size limit in bytes rather than characters, so multibyte bodies cannot exceed the configured limit on disk - Subtract raw float timestamps before rounding durations, removing the +-1ms truncation error; still stored as whole milliseconds --- README.md | 2 +- config/barstool.php | 6 ++- src/Barstool.php | 24 +++++++-- src/BarstoolServiceProvider.php | 4 -- tests/BarstoolTest.php | 89 +++++++++++++++++++++++++++++++++ 5 files changed, 113 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index cd1f59b..fd2d8e1 100644 --- a/README.md +++ b/README.md @@ -114,7 +114,7 @@ If you mainly use Barstool to investigate failures, you can skip storing success 'keep_successful_responses' => false, ``` -The request itself is still recorded — you just won't get the response body, headers, and status for successful calls. Failed responses and fatal errors are always kept. +The request, response status, duration, and outcome are still recorded for successful calls — only the response body and headers are omitted, so your "show me failures" queries stay accurate. Failed responses and fatal errors are always kept in full. ### Redacting sensitive request headers diff --git a/config/barstool.php b/config/barstool.php index 8449969..328b0c3 100644 --- a/config/barstool.php +++ b/config/barstool.php @@ -25,8 +25,10 @@ 'max_response_size' => 100, /* - * Indicates if successful responses should be kept. - * If false, only failed responses will be kept however the request will still be recorded. + * Indicates if successful response bodies and headers should be kept. + * If false, the request, status, duration and outcome are still recorded + * for successful responses - only the response body and headers are omitted. + * Failed responses are always kept in full. */ 'keep_successful_responses' => true, diff --git a/src/Barstool.php b/src/Barstool.php index c01f3f4..5fe956d 100755 --- a/src/Barstool.php +++ b/src/Barstool.php @@ -257,9 +257,20 @@ private static function recordResponse(Response $data): void return; } + // When successful responses are not kept, still record the outcome + // (status, successful, duration) so the row does not read as a failure - + // only the body and headers are omitted. + $keepResponse = $data->failed() || config('barstool.keep_successful_responses') !== false; + $payload = [ 'duration' => self::calculateDuration($data), - ...self::getResponseData($data), + ...($keepResponse ? self::getResponseData($data) : [ + 'url' => $data->getPsrRequest()->getUri(), + 'response_headers' => null, + 'response_body' => null, + 'response_status' => $data->status(), + 'successful' => $data->successful(), + ]), ]; self::persist(RecordingType::RESPONSE, $payload, $uuid); @@ -273,10 +284,12 @@ public static function calculateDuration(Response|PendingRequest $data): int ? $data->getPendingRequest()->config() : $data->config(); - $requestTime = (int) $config->get('barstool-request-time'); - $responseTime = (int) $config->get('barstool-response-time', microtime(true) * 1000); + // Subtract the raw float timestamps before rounding - truncating each + // timestamp first introduces up to +-1ms of error on the duration. + $requestTime = (float) $config->get('barstool-request-time'); + $responseTime = (float) $config->get('barstool-response-time', microtime(true) * 1000); - return $responseTime - $requestTime; + return (int) round($responseTime - $requestTime); } private static function recordFatal(FatalRequestException $data): void @@ -423,7 +436,8 @@ private static function checkContentSize(mixed $body): bool try { $body = (string) $body; - return intdiv(mb_strlen($body), 1000) <= config('barstool.max_response_size', 100); + // strlen, not mb_strlen: the limit is about storage size, so count bytes + return intdiv(strlen($body), 1000) <= config('barstool.max_response_size', 100); } catch (\Throwable) { return false; } diff --git a/src/BarstoolServiceProvider.php b/src/BarstoolServiceProvider.php index 0e708f8..3f0a78e 100644 --- a/src/BarstoolServiceProvider.php +++ b/src/BarstoolServiceProvider.php @@ -60,10 +60,6 @@ public function packageRegistered(): void microtime(true) * 1000 ); - if ($response->successful() && config('barstool.keep_successful_responses') === false) { - return; - } - Barstool::record($response); }); } diff --git a/tests/BarstoolTest.php b/tests/BarstoolTest.php index 14c5ff3..97152e8 100644 --- a/tests/BarstoolTest.php +++ b/tests/BarstoolTest.php @@ -1171,3 +1171,92 @@ expect($schema->hasIndex('barstools', 'barstools_created_at_index'))->toBeTrue(); }); + +it('records the outcome of successful responses when keep_successful_responses is false', function () { + config()->set('barstool.enabled', true); + config()->set('barstool.keep_successful_responses', false); + + MockClient::global([ + SoloUserRequest::class => MockResponse::make( + body: ['data' => 'ok'], + status: 200, + headers: ['token' => 'abc123'], + ), + ]); + + (new SoloUserRequest)->send(); + + $barstool = Barstool::sole(); + + expect($barstool) + ->successful->toBeTrue() + ->response_status->toBe(200) + ->duration->not->toBeNull() + ->response_body->toBeNull() + ->response_headers->toBeNull(); +}); + +it('keeps failed responses in full when keep_successful_responses is false', function () { + config()->set('barstool.enabled', true); + config()->set('barstool.keep_successful_responses', false); + + MockClient::global([ + SoloUserRequest::class => MockResponse::make( + body: ['error' => 'whoops'], + status: 500, + headers: ['Content-Type' => 'application/json'], + ), + ]); + + (new SoloUserRequest)->send(); + + $barstool = Barstool::sole(); + + expect($barstool) + ->successful->toBeFalse() + ->response_status->toBe(500) + ->response_body->toBe(json_encode(['error' => 'whoops'])); +}); + +it('measures the response size limit in bytes rather than characters', function () { + config()->set('barstool.enabled', true); + config()->set('barstool.max_response_size', 1); + + // 700 three-byte characters: 700 chars but 2100 bytes - over a 1KB byte limit + MockClient::global([ + SoloUserRequest::class => MockResponse::make( + body: str_repeat('€', 700), + status: 200, + headers: ['Content-Type' => 'text/plain'], + ), + ]); + + (new SoloUserRequest)->send(); + + expect(Barstool::sole()->response_body)->toBe(''); +}); + +it('rounds durations to the nearest whole millisecond', function () { + config()->set('barstool.enabled', true); + + $pendingRequest = (new SoloUserRequest)->createPendingRequest(); + + $pendingRequest->config()->add('barstool-request-time', 1000.3); + $pendingRequest->config()->add('barstool-response-time', 1001.7); + + // Truncate-then-subtract would give 1; correct rounding gives 1 too, + // but 1000.6 -> 1002.4 shows the difference: + expect(BarstoolRecorder::calculateDuration($pendingRequest))->toBe(1); + + $pendingRequest->config()->add('barstool-request-time', 1000.6); + $pendingRequest->config()->add('barstool-response-time', 1002.4); + + // Old behaviour: (int) 1002.4 - (int) 1000.6 = 2ms error path (1002-1000); + // correct: round(1.8) = 2 -- and a sub-ms request no longer records 0 + expect(BarstoolRecorder::calculateDuration($pendingRequest))->toBe(2); + + $pendingRequest->config()->add('barstool-request-time', 1000.2); + $pendingRequest->config()->add('barstool-response-time', 1000.9); + + expect(BarstoolRecorder::calculateDuration($pendingRequest))->toBe(1); +});