From fccff18c4e7c70fd160be08e59b4d66cfef8d352 Mon Sep 17 00:00:00 2001 From: Andres Contreras Date: Tue, 13 Oct 2026 16:49:57 -0700 Subject: [PATCH 1/5] feat(kernel): give FireflyException a public detail distinct from its message ErrorResponse::fromException() now publishes detail(), which is getMessage() by default. withDetail(?string) replaces it on the instance without touching the message the log reads; null means the exception holds no sentence written for a client. --- packages/kernel/src/Error/ErrorResponse.php | 4 +- .../kernel/src/Exception/FireflyException.php | 40 +++++++++++++++++++ .../kernel/tests/Error/ErrorResponseTest.php | 9 +++++ .../tests/Exception/FireflyExceptionTest.php | 14 +++++++ 4 files changed, 66 insertions(+), 1 deletion(-) diff --git a/packages/kernel/src/Error/ErrorResponse.php b/packages/kernel/src/Error/ErrorResponse.php index 8cc3033e..866edddb 100644 --- a/packages/kernel/src/Error/ErrorResponse.php +++ b/packages/kernel/src/Error/ErrorResponse.php @@ -83,7 +83,9 @@ public static function fromException( code: $e->errorCode(), category: $e->category(), severity: $e->severity(), - detail: $e->getMessage(), + // detail(), not getMessage(): a wrapper's message is the log's line, and its detail is the + // cause's own sentence or null (the member is then left out) — see FireflyException. + detail: $e->detail(), type: $type, instance: $instance, traceId: $traceId, diff --git a/packages/kernel/src/Exception/FireflyException.php b/packages/kernel/src/Exception/FireflyException.php index 9cb48e8d..4cb932b9 100644 --- a/packages/kernel/src/Exception/FireflyException.php +++ b/packages/kernel/src/Exception/FireflyException.php @@ -35,6 +35,18 @@ * over the extensions, so a `['status' => 999]` extension is harmless rather than a way to lie about the * status. That rule is enforced where the document is built, not here, because the document is the only * place where the two namespaces meet. + * + * AND THE SENTENCE FOR THE CLIENT IS NOT ALWAYS THE MESSAGE. `detail()` is what the problem document + * publishes as `detail`, and by default it is getMessage(), because a message in this taxonomy is written + * for the client ("Order 42 does not exist."). A WRAPPER breaks that: CQRS's CommandProcessingException + * reads "Processing command [App\Withdraw] failed: ", which is the right line for a + * log and the wrong one for a wire — it names a class, and when the cause is a QueryException it carries + * the failing SQL and its bindings. So a wrapper says what may be published with `withDetail()`: the + * cause's own detail when the cause is authored, and NULL when nothing it holds was written for a client. + * Null is not "empty": the web layer reads it as "treat this like an unhandled throwable" and publishes + * its opaque sentence (or, with disclosure on, the underlying cause's message), keeping this exception's + * code, status, category, severity, title and extension members. getMessage() is never touched, so the + * log and the `previous` chain keep every word. */ class FireflyException extends RuntimeException { @@ -43,6 +55,10 @@ class FireflyException extends RuntimeException private ?string $title; + private ?string $detail = null; + + private bool $detailAuthored = false; + /** * @param array $extensions RFC 9457 extension members spread into the problem document * @param ?string $title the problem's own short phrase; null means the status's reason phrase @@ -122,4 +138,28 @@ public function withTitle(?string $title): static return $this; } + + /** + * The sentence the problem document publishes as `detail`: getMessage() unless withDetail() said + * otherwise, and null when this exception carries no sentence written for a client — see the class + * comment for what the web layer does then. + */ + public function detail(): ?string + { + return $this->detailAuthored ? $this->detail : $this->getMessage(); + } + + /** + * Replaces the published sentence on THIS instance — null withholds it — without touching getMessage(), + * which stays the log's. + * + * @return $this + */ + public function withDetail(?string $detail): static + { + $this->detail = $detail; + $this->detailAuthored = true; + + return $this; + } } diff --git a/packages/kernel/tests/Error/ErrorResponseTest.php b/packages/kernel/tests/Error/ErrorResponseTest.php index 6f8de398..d50eb365 100644 --- a/packages/kernel/tests/Error/ErrorResponseTest.php +++ b/packages/kernel/tests/Error/ErrorResponseTest.php @@ -151,3 +151,12 @@ expect(array_keys($payload))->toBe(['status', 'title', 'code', 'category', 'severity', 'detail']); }); + +it('publishes the exception\'s detail rather than its message, and leaves the member out when it is withheld', function () { + $replaced = ErrorResponse::fromException((new ResourceNotFoundException('Loading [App\Order] failed: Order 42 not found'))->withDetail('Order 42 not found')); + $withheld = ErrorResponse::fromException((new ResourceNotFoundException('select * from orders where id = 42'))->withDetail(null)); + + expect($replaced->detail)->toBe('Order 42 not found') + ->and($withheld->detail)->toBeNull() + ->and($withheld->toArray())->not->toHaveKey('detail'); +}); diff --git a/packages/kernel/tests/Exception/FireflyExceptionTest.php b/packages/kernel/tests/Exception/FireflyExceptionTest.php index 8affd6b6..ee3c0fe8 100644 --- a/packages/kernel/tests/Exception/FireflyExceptionTest.php +++ b/packages/kernel/tests/Exception/FireflyExceptionTest.php @@ -62,3 +62,17 @@ expect($e->extensions())->toBe(['retryable' => true]) ->and($e->title())->toBe('That cannot be done yet'); }); + +it('publishes its message as the detail unless told otherwise', function () { + expect((new FireflyException('Order 42 does not exist.', 'NOT_FOUND', 404))->detail())->toBe('Order 42 does not exist.'); +}); + +it('can replace or withhold the published detail without touching the message the log reads', function () { + $replaced = (new FireflyException('Wrapping [App\Withdraw] failed: Overdrawn.', 'X', 409))->withDetail('Overdrawn.'); + $withheld = (new FireflyException('Wrapping [App\Withdraw] failed: select * from users', 'X'))->withDetail(null); + + expect($replaced->detail())->toBe('Overdrawn.') + ->and($replaced->getMessage())->toBe('Wrapping [App\Withdraw] failed: Overdrawn.') + ->and($withheld->detail())->toBeNull() + ->and($withheld->getMessage())->toBe('Wrapping [App\Withdraw] failed: select * from users'); +}); From 6ffcc1f60f06d1f115e7c237171b5f791c144475 Mon Sep 17 00:00:00 2001 From: Andres Contreras Date: Tue, 13 Oct 2026 16:49:58 -0700 Subject: [PATCH 2/5] fix(web): map a FireflyException with a withheld detail like an unhandled throwable A FireflyException whose detail() is null keeps its code, status, category, severity, title and extension members, but its sentence is the opaque one (with the reference) unless the deployment discloses, and then the message of the throwable underneath every such wrapper. The HTML page's authored detail reads detail() instead of getMessage(). --- packages/web/src/Error/ProblemMapper.php | 40 ++++++++- .../Error/ProblemMapperWithheldDetailTest.php | 86 +++++++++++++++++++ 2 files changed, 125 insertions(+), 1 deletion(-) create mode 100644 packages/web/tests/Error/ProblemMapperWithheldDetailTest.php diff --git a/packages/web/src/Error/ProblemMapper.php b/packages/web/src/Error/ProblemMapper.php index 689c3f48..835231ab 100644 --- a/packages/web/src/Error/ProblemMapper.php +++ b/packages/web/src/Error/ProblemMapper.php @@ -45,6 +45,14 @@ * sentence and its real message stays in the exception, where the log has it. When the caller knows the * request's correlation id it passes it as `$reference`, and the fixed sentence names it — so the one thing * a person can do with an opaque 5xx, quote it, is possible from the body alone. + * + * A WRAPPER WITH NOTHING AUTHORED IS THE SAME ACCIDENT IN A FIREFLY COAT. DefaultCommandBus wraps a handler's + * QueryException in a CommandProcessingException — a FireflyException, so the first case used to return it + * untouched and its message, "Processing command [App\Withdraw] failed: ", went + * out as `detail` with disclosure off. A FireflyException whose detail() is null is therefore mapped like + * an unhandled throwable FOR THE SENTENCE ONLY — opaque unless `$disclose`, and then the message of the + * throwable it wraps — while its own code, status, category, severity, title and extension members are + * kept, because those were never the problem. The wrapper stays `previous`, so the log loses nothing. */ final class ProblemMapper { @@ -67,6 +75,7 @@ final class ProblemMapper public static function toFireflyException(Throwable $e, bool $disclose = true, ?string $reference = null): FireflyException { return match (true) { + $e instanceof FireflyException && $e->detail() === null => self::withheld($e, $disclose, $reference), $e instanceof FireflyException => $e, self::isExecutionTimeLimit($e) => self::executionTimeExceeded($e, $reference), $e instanceof MethodNotAllowedHttpException => self::methodNotAllowed($e), @@ -125,7 +134,9 @@ public static function authoredDetail(Throwable $e): string $mapped = self::toFireflyException($e, disclose: false); - return $mapped->httpStatus() < 500 ? $mapped->getMessage() : ''; + // detail(), not getMessage(): over the CQRS bus the message names the command class in front of the + // sentence, and the page says what the problem document says. + return $mapped->httpStatus() < 500 ? ($mapped->detail() ?? '') : ''; } /** @@ -327,6 +338,33 @@ private static function executionTimeExceeded(Throwable $e, ?string $reference): ); } + /** + * A FireflyException with no sentence for a client, re-stated with one: the opaque sentence, or — when + * the deployment discloses — the message of the throwable underneath every such wrapper, which is what + * that throwable would have published had a controller thrown it. Everything but the sentence is the + * wrapper's own. + */ + private static function withheld(FireflyException $e, bool $disclose, ?string $reference): FireflyException + { + $cause = $e; + while ($cause instanceof FireflyException && $cause->detail() === null && $cause->getPrevious() !== null) { + $cause = $cause->getPrevious(); + } + + $message = $cause instanceof FireflyException ? ($cause->detail() ?? $cause->getMessage()) : $cause->getMessage(); + + return new FireflyException( + $disclose && $message !== '' ? $message : self::opaque($reference), + $e->errorCode(), + $e->httpStatus(), + $e->category(), + $e->severity(), + $e, + $e->extensions(), + $e->title(), + ); + } + private static function opaque(?string $reference): string { return $reference === null ? self::OPAQUE : sprintf(self::OPAQUE_WITH_REFERENCE, $reference); diff --git a/packages/web/tests/Error/ProblemMapperWithheldDetailTest.php b/packages/web/tests/Error/ProblemMapperWithheldDetailTest.php new file mode 100644 index 00000000..4dad0db6 --- /dev/null +++ b/packages/web/tests/Error/ProblemMapperWithheldDetailTest.php @@ -0,0 +1,86 @@ +getMessage()}", $code, 500, ErrorCategory::Internal, ErrorSeverity::Error, $cause)) + ->withDetail(null); +} + +it('answers a withheld detail with the opaque sentence, naming the reference when there is one', function () { + $wrapper = withheldWrapper(new RuntimeException('select * from users where token = tok')); + + $plain = ProblemMapper::toFireflyException($wrapper, disclose: false); + $referenced = ProblemMapper::toFireflyException($wrapper, disclose: false, reference: 'ref-1'); + + expect($plain->detail())->toBe(ProblemMapper::OPAQUE) + ->and($referenced->detail())->toBe(sprintf(ProblemMapper::OPAQUE_WITH_REFERENCE, 'ref-1')) + // The wrapper is `previous`, so the log keeps the class name and the cause's message. + ->and($plain->getPrevious())->toBe($wrapper); +}); + +it('keeps the wrapper\'s code, status, category, severity, title and extension members', function () { + $wrapper = (new FireflyException('Processing query [App\FindOrder] failed: boom', 'QUERY_PROCESSING_ERROR', 503, ErrorCategory::Infrastructure, ErrorSeverity::Critical, new RuntimeException('boom'))) + ->withExtensions(['retryable' => true]) + ->withTitle('Ledger offline') + ->withDetail(null); + + $mapped = ProblemMapper::toFireflyException($wrapper, disclose: false); + + expect($mapped->errorCode())->toBe('QUERY_PROCESSING_ERROR') + ->and($mapped->httpStatus())->toBe(503) + ->and($mapped->category())->toBe(ErrorCategory::Infrastructure) + ->and($mapped->severity())->toBe(ErrorSeverity::Critical) + ->and($mapped->extensions())->toBe(['retryable' => true]) + ->and($mapped->title())->toBe('Ledger offline'); +}); + +it('publishes the innermost cause\'s own message, without any wrapper\'s prefix, when the deployment discloses', function () { + $cause = new RuntimeException('select secret'); + $nested = withheldWrapper(withheldWrapper($cause, 'QUERY_PROCESSING_ERROR')); + + expect(ProblemMapper::toFireflyException($nested, disclose: true)->detail())->toBe('select secret') + ->and(ProblemMapper::toFireflyException($nested, disclose: false)->detail())->toBe(ProblemMapper::OPAQUE); +}); + +it('falls back to the opaque sentence when the disclosed cause has nothing to say', function () { + expect(ProblemMapper::toFireflyException(withheldWrapper(new RuntimeException('')), disclose: true)->detail()) + ->toBe(ProblemMapper::OPAQUE); +}); + +it('puts a wrapper\'s published detail on the HTML page, never its message', function () { + $wrapper = (new ConflictException('Processing command [App\Withdraw] failed: The wallet cannot be overdrawn.')) + ->withDetail('The wallet cannot be overdrawn.'); + + expect(ProblemMapper::authoredDetail($wrapper))->toBe('The wallet cannot be overdrawn.'); +}); + +it('renders a withheld detail through the problem renderer exactly as it renders an unhandled throwable', function () { + $request = Request::create('/api/x', 'POST'); + $request->headers->set('X-Correlation-Id', 'corr-7'); + + /** @var array $fromWrapper */ + $fromWrapper = json_decode((string) (new ProblemDetailsRenderer)->render(withheldWrapper(new RuntimeException('select secret')), $request)->getContent(), true); + /** @var array $fromThrowable */ + $fromThrowable = json_decode((string) (new ProblemDetailsRenderer)->render(new RuntimeException('select secret'), $request)->getContent(), true); + + expect($fromWrapper['detail'] ?? null)->toBe(sprintf(ProblemMapper::OPAQUE_WITH_REFERENCE, 'corr-7')) + ->and($fromWrapper['detail'] ?? null)->toBe($fromThrowable['detail'] ?? null) + ->and($fromWrapper['code'] ?? null)->toBe('COMMAND_PROCESSING_ERROR'); +}); From 277cbc974a029b15426a6d19e70cb5b5be4768c1 Mon Sep 17 00:00:00 2001 From: Andres Contreras Date: Tue, 13 Oct 2026 16:49:58 -0700 Subject: [PATCH 3/5] fix(cqrs): stop publishing a handler's raw cause and the message class as detail Command/QueryProcessingException messages read "Processing command [] failed: ", and that message was the problem document's detail: a handler's QueryException put its SQL and bindings on the wire with disclosure off, and an authored refusal carried the class name in front of its sentence. The wrappers now publish the cause's own detail(), or null when the cause is not a FireflyException, so the document matches the one the same throwable gets from a controller. The message keeps the prefix for the log. --- .../Exception/CommandProcessingException.php | 12 +- .../Exception/QueryProcessingException.php | 12 +- .../Exception/CqrsExceptionTaxonomyTest.php | 33 +++ .../Cqrs/BusFailureProblemDocumentTest.php | 214 ++++++++++++++++++ .../Cqrs/BusRefusalProblemDocumentTest.php | 14 +- .../tests/Fixtures/Cqrs/FailingCommand.php | 11 + .../Fixtures/Cqrs/FailingCommandHandler.php | 13 ++ .../tests/Fixtures/Cqrs/FailingQuery.php | 11 + .../Fixtures/Cqrs/FailingQueryHandler.php | 13 ++ .../security/tests/Fixtures/Cqrs/Failures.php | 37 +++ samples/lumen/tests/Web/WalletRestTest.php | 5 +- 11 files changed, 367 insertions(+), 8 deletions(-) create mode 100644 packages/security/tests/Cqrs/BusFailureProblemDocumentTest.php create mode 100644 packages/security/tests/Fixtures/Cqrs/FailingCommand.php create mode 100644 packages/security/tests/Fixtures/Cqrs/FailingCommandHandler.php create mode 100644 packages/security/tests/Fixtures/Cqrs/FailingQuery.php create mode 100644 packages/security/tests/Fixtures/Cqrs/FailingQueryHandler.php create mode 100644 packages/security/tests/Fixtures/Cqrs/Failures.php diff --git a/packages/cqrs/src/Exception/CommandProcessingException.php b/packages/cqrs/src/Exception/CommandProcessingException.php index 92cd7763..db1d0245 100644 --- a/packages/cqrs/src/Exception/CommandProcessingException.php +++ b/packages/cqrs/src/Exception/CommandProcessingException.php @@ -15,7 +15,8 @@ * FireflyException, its errorCode/httpStatus/category/severity are copied so an expected client/domain fault (validation, * not-found) keeps its own kernel category and is NOT masked into a generic 500, and its RFC 9457 extension members and * title are copied so the problem document says what the cause would have said on its own; a plain Throwable becomes - * internal/500. Carries the command class + the cause as `previous` (design §2.3 / pyfly command/bus.py:153-164). + * internal/500. The message keeps the command class and the cause's message for the log; the published detail() is + * the cause's own detail, or null (opaque on the wire) when the cause is not a FireflyException. Carries the command class + the cause as `previous` (design §2.3 / pyfly command/bus.py:153-164). */ final class CommandProcessingException extends CqrsException { @@ -46,5 +47,14 @@ public function __construct(string $commandClass, Throwable $cause) $cause instanceof FireflyException ? $cause->extensions() : [], $cause instanceof FireflyException ? $cause->title() : null, ); + + /* + | And what the client may read, which is NOT the message above. The message names the command + | class and quotes the cause verbatim — the log's line. On the wire an authored cause says its own + | sentence, exactly as it would thrown from a controller; anything else (a QueryException with its + | SQL and bindings, a TypeError naming a path) has no sentence for a client, so the detail is null + | and the web layer answers with its opaque one, as it does for the same throwable unwrapped. + */ + $this->withDetail($cause instanceof FireflyException ? $cause->detail() : null); } } diff --git a/packages/cqrs/src/Exception/QueryProcessingException.php b/packages/cqrs/src/Exception/QueryProcessingException.php index 8a40ec1a..a8eb3cc8 100644 --- a/packages/cqrs/src/Exception/QueryProcessingException.php +++ b/packages/cqrs/src/Exception/QueryProcessingException.php @@ -12,7 +12,8 @@ /** * The query-side twin of CommandProcessingException: DefaultQueryBus wraps any handler/stage throwable in this * (unless already one), preserving a FireflyException cause's code/httpStatus/category/severity and its RFC 9457 - * extension members and title, else internal/500. + * extension members and title, else internal/500. Its detail() is the cause's own, or null when the cause is not a + * FireflyException, so the query class and a raw cause message stay in the log. */ final class QueryProcessingException extends CqrsException { @@ -43,5 +44,14 @@ public function __construct(string $queryClass, Throwable $cause) $cause instanceof FireflyException ? $cause->extensions() : [], $cause instanceof FireflyException ? $cause->title() : null, ); + + /* + | And what the client may read, which is NOT the message above. The message names the query + | class and quotes the cause verbatim — the log's line. On the wire an authored cause says its own + | sentence, exactly as it would thrown from a controller; anything else (a QueryException with its + | SQL and bindings, a TypeError naming a path) has no sentence for a client, so the detail is null + | and the web layer answers with its opaque one, as it does for the same throwable unwrapped. + */ + $this->withDetail($cause instanceof FireflyException ? $cause->detail() : null); } } diff --git a/packages/cqrs/tests/Exception/CqrsExceptionTaxonomyTest.php b/packages/cqrs/tests/Exception/CqrsExceptionTaxonomyTest.php index e1dc7141..50463cb0 100644 --- a/packages/cqrs/tests/Exception/CqrsExceptionTaxonomyTest.php +++ b/packages/cqrs/tests/Exception/CqrsExceptionTaxonomyTest.php @@ -116,3 +116,36 @@ 'query, plain cause' => [QueryProcessingException::class, new RuntimeException('boom')], 'command, bare FireflyException' => [CommandProcessingException::class, new ResourceNotFoundException('gone')], ]); + +/* + | The MESSAGE names the message class and quotes the cause — that is the log's line. The DETAIL is what the + | problem document publishes, and it is the cause's own: its sentence when it is a FireflyException, and + | nothing (null — the web layer's opaque sentence) when it is not. + */ +it('publishes an authored cause\'s own detail and keeps the message class in the message alone', function (string $wrapper) { + $cause = new ResourceNotFoundException('Order 42 does not exist.'); + /** @var FireflyException $wrapped */ + $wrapped = new $wrapper('App\Order', $cause); + + expect($wrapped->detail())->toBe('Order 42 does not exist.') + ->and($wrapped->getMessage())->toContain('App\Order') + ->and($wrapped->getMessage())->toEndWith('failed: Order 42 does not exist.'); +})->with([CommandProcessingException::class, QueryProcessingException::class]); + +it('withholds the detail when the cause is not a FireflyException', function (string $wrapper) { + $cause = new RuntimeException('SQLSTATE[HY000] (SQL: select * from users where token = tok)'); + /** @var FireflyException $wrapped */ + $wrapped = new $wrapper('App\Order', $cause); + + expect($wrapped->detail())->toBeNull() + ->and($wrapped->getMessage())->toContain('select * from users'); +})->with([CommandProcessingException::class, QueryProcessingException::class]); + +it('carries a cause\'s own replaced or withheld detail through, so a wrapper of a wrapper says what the innermost may', function () { + $inner = new QueryProcessingException('App\FindOrder', new RuntimeException('select secret')); + $outer = new CommandProcessingException('App\PlaceOrder', $inner); + $authored = new CommandProcessingException('App\PlaceOrder', (new FireflyException('raw', 'X', 409))->withDetail('Said for the client.')); + + expect($outer->detail())->toBeNull() + ->and($authored->detail())->toBe('Said for the client.'); +}); diff --git a/packages/security/tests/Cqrs/BusFailureProblemDocumentTest.php b/packages/security/tests/Cqrs/BusFailureProblemDocumentTest.php new file mode 100644 index 00000000..ea771778 --- /dev/null +++ b/packages/security/tests/Cqrs/BusFailureProblemDocumentTest.php @@ -0,0 +1,214 @@ +] failed: ". Until the kernel grew a + | public-detail seam that message WAS the problem document's `detail`, so a handler's QueryException put + | its SQL and bindings on the wire — in production, with disclosure off — where the same throwable raised + | by a controller answered with the opaque sentence, and an authored refusal carried the command's class + | name in front of the sentence the application wrote for its client. + | + | These drive both seams through the real HTTP pipeline — WebServiceProvider's renderable, the real + | buses resolved from the application context, a compiled HandlerManifest — and compare what a client + | receives. + */ + +abstract class BusFailureTestCase extends FireflyTestCase +{ + protected function fireflyProviders(): array + { + return [ + ValidationServiceProvider::class, + WebServiceProvider::class, + CqrsServiceProvider::class, + CqrsWiringProvider::class, + ]; + } + + protected function configOverrides(): array + { + return ['app.debug' => false, 'firefly.web.problem.disclose' => false]; + } + + /** Turns `firefly.web.problem.disclose` on — what a debugging deployment sets — for the rest of the test. */ + public function disclose(): void + { + config()->set('firefly.web.problem.disclose', true); + $this->app()->forgetInstance(ErrorPageSettings::class); + $this->app()->forgetInstance(ProblemDetailsRenderer::class); + } + + protected function defineFireflyEnvironment(Application $app): void + { + $app->instance(HandlerManifest::class, new HandlerManifest([ + new HandlerDescriptor(FailingCommand::class, FailingCommandHandler::class, 'handle', HandlerKind::Command), + new HandlerDescriptor(FailingQuery::class, FailingQueryHandler::class, 'handle', HandlerKind::Query), + ], [])); + } + + protected function setUp(): void + { + parent::setUp(); + + Route::post('/controller/{failure}', static fn (string $failure): never => throw Failures::make($failure)); + Route::post('/command/{failure}', static function (string $failure, ApplicationContext $context): mixed { + /** @var CommandBus $bus */ + $bus = $context->get(CommandBus::class); + + return $bus->send(new FailingCommand($failure)); + }); + Route::post('/query/{failure}', static function (string $failure, ApplicationContext $context): mixed { + /** @var QueryBus $bus */ + $bus = $context->get(QueryBus::class); + + return $bus->ask(new FailingQuery($failure)); + }); + } + + /** + * The problem document a JSON client receives, and its raw body. + * + * @return array{0: array, 1: string} + */ + public function problem(string $path): array + { + $response = $this->postJson($path); + $response->assertHeader('Content-Type', 'application/problem+json'); + + /** @var array $document */ + $document = $response->json(); + + return [$document, (string) $response->getContent()]; + } + + /** @return TestResponse */ + public function page(string $path): TestResponse + { + return $this->post($path, [], ['Accept' => 'text/html']); + } +} + +/** + * The document minus the members that describe the request rather than the failure. + * + * @param array $document + * @return array + */ +function busFailureIdentity(array $document, string ...$also): array +{ + return array_diff_key($document, array_flip(['instance', 'traceId', 'correlationId', 'timestamp', ...$also])); +} + +uses(BusFailureTestCase::class); + +/** + * The opaque sentence naming the document's own reference. + * + * @param array $document + */ +function busFailureOpaque(array $document): string +{ + $reference = $document['traceId'] ?? null; + + return sprintf(ProblemMapper::OPAQUE_WITH_REFERENCE, is_string($reference) ? $reference : ''); +} + +describe('with disclosure off (production)', function () { + it('answers an unhandled handler throwable with the opaque sentence a controller gets, never its message or the message class', function (string $seam, string $failure, string $code) { + /** @var BusFailureTestCase $this */ + [$fromController] = $this->problem("/controller/{$failure}"); + [$fromBus, $raw] = $this->problem("/{$seam}/{$failure}"); + + expect($fromBus['status'])->toBe(500) + ->and($fromBus['code'])->toBe($code) + ->and($fromBus['detail'])->toBe(busFailureOpaque($fromBus)) + ->and($fromController['detail'])->toBe(busFailureOpaque($fromController)) + // The wrapper keeps its own code (and the `type` derived from it); everything else is the controller's. + ->and(busFailureIdentity($fromBus, 'detail', 'code', 'type'))->toBe(busFailureIdentity($fromController, 'detail', 'code', 'type')); + + foreach (['SELECT', 'select', 'secret', Failures::BINDING, 'no such table', 'Processing', 'FailingCommand', 'FailingQuery', 'Fixtures'] as $leak) { + expect($raw)->not->toContain($leak); + } + })->with([ + 'a RuntimeException from a command' => ['command', 'runtime', 'COMMAND_PROCESSING_ERROR'], + 'a QueryException from a command' => ['command', 'query', 'COMMAND_PROCESSING_ERROR'], + 'a RuntimeException from a query' => ['query', 'runtime', 'QUERY_PROCESSING_ERROR'], + 'a QueryException from a query' => ['query', 'query', 'QUERY_PROCESSING_ERROR'], + ]); + + it('publishes an authored cause exactly as a controller does, detail included', function (string $seam, string $failure, int $status, string $detail) { + /** @var BusFailureTestCase $this */ + [$fromController] = $this->problem("/controller/{$failure}"); + [$fromBus, $raw] = $this->problem("/{$seam}/{$failure}"); + + expect(busFailureIdentity($fromBus))->toBe(busFailureIdentity($fromController)) + ->and($fromBus['status'])->toBe($status) + ->and($fromBus['detail'])->toBe($detail) + ->and($raw)->not->toContain('Processing') + ->and($raw)->not->toContain('Fixtures'); + })->with([ + 'a 409 from a command' => ['command', 'conflict', 409, 'The wallet cannot be overdrawn.'], + 'a 409 from a query' => ['query', 'conflict', 409, 'The wallet cannot be overdrawn.'], + 'an authored 503 from a command' => ['command', 'unavailable', 503, 'The ledger is closed for the night; try again after 06:00.'], + 'an authored 503 from a query' => ['query', 'unavailable', 503, 'The ledger is closed for the night; try again after 06:00.'], + ]); + + it('puts the cause\'s own sentence on the HTML page for an authored refusal, and nothing of an unhandled one', function () { + /** @var BusFailureTestCase $this */ + $authored = $this->page('/command/conflict')->assertStatus(409); + $unhandled = $this->page('/command/query')->assertStatus(500); + + expect((string) $authored->getContent())->toContain('The wallet cannot be overdrawn.') + ->and((string) $authored->getContent())->not->toContain('Processing command'); + + foreach (['from users', Failures::BINDING, 'no such table', 'Processing', 'FailingCommand'] as $leak) { + expect((string) $unhandled->getContent())->not->toContain($leak); + } + }); +}); + +describe('with disclosure on (debug)', function () { + it('publishes the cause\'s own message, as a controller does, without the message class in front of it', function (string $seam, string $failure) { + /** @var BusFailureTestCase $this */ + $this->disclose(); + [$fromController] = $this->problem("/controller/{$failure}"); + [$fromBus] = $this->problem("/{$seam}/{$failure}"); + + expect($fromBus['status'])->toBe(500) + ->and($fromBus['detail'])->toBe(Failures::make($failure)->getMessage()) + ->and($fromBus['detail'])->toBe($fromController['detail']); + })->with([ + 'a RuntimeException from a command' => ['command', 'runtime'], + 'a QueryException from a command' => ['command', 'query'], + 'a RuntimeException from a query' => ['query', 'runtime'], + 'a QueryException from a query' => ['query', 'query'], + ]); +}); diff --git a/packages/security/tests/Cqrs/BusRefusalProblemDocumentTest.php b/packages/security/tests/Cqrs/BusRefusalProblemDocumentTest.php index 95b13692..8611ac1a 100644 --- a/packages/security/tests/Cqrs/BusRefusalProblemDocumentTest.php +++ b/packages/security/tests/Cqrs/BusRefusalProblemDocumentTest.php @@ -42,8 +42,10 @@ | the cause's code, status, category and severity but not its extension members, so the same rule, refusing | the same principal, published `requiredAuthorities` from a controller and dropped it from a command. | - | These render both refusals through the real renderer and compare the documents. `detail` is the one member - | allowed to differ: the bus prefixes the cause's sentence with the command it was processing. + | These render both refusals through the real renderer and compare the documents, `detail` included. The + | bus's own message still names the command it was processing ("Processing command [...] failed: ..."), but + | that line is the log's: the wrapper's detail() is the cause's own sentence, so nothing but the members + | that describe the request may differ. */ uses(TestCase::class); @@ -143,7 +145,7 @@ function busRefusalBothWays(string $expression, ?string $code = null, ?string $m */ function busRefusalIdentity(array $document): array { - return array_diff_key($document, array_flip(['detail', 'instance', 'traceId', 'correlationId', 'timestamp'])); + return array_diff_key($document, array_flip(['instance', 'traceId', 'correlationId', 'timestamp'])); } it('publishes requiredAuthorities for a refused command exactly as it does for a refused controller method', function () { @@ -159,7 +161,7 @@ function busRefusalIdentity(array $document): array ->and($fromBus['code'])->toBe('ACCESS_DENIED'); }); -it('keeps the rule\'s own code and sentence on a refused command, and only prefixes the sentence', function () { +it('keeps the rule\'s own code and sentence on a refused command, word for word', function () { [$controller, $bus] = busRefusalBothWays("hasAnyRole('MANAGER', 'TENANT_ADMIN')", 'RUN_ROLE_REQUIRED', 'Only a manager may start a run.'); $fromController = busRefusalDocument($controller); @@ -169,5 +171,7 @@ function busRefusalIdentity(array $document): array ->and($fromBus['code'])->toBe('RUN_ROLE_REQUIRED') ->and($fromBus['requiredAuthorities'] ?? null)->toBe(['ROLE_MANAGER', 'ROLE_TENANT_ADMIN']) ->and($fromController['detail'])->toBe('Only a manager may start a run.') - ->and($fromBus['detail'])->toBe('Processing command ['.AdminCommand::class.'] failed: Only a manager may start a run.'); + ->and($fromBus['detail'])->toBe('Only a manager may start a run.') + // The prefix is the log's, and it stays on the message. + ->and($bus->getMessage())->toBe('Processing command ['.AdminCommand::class.'] failed: Only a manager may start a run.'); }); diff --git a/packages/security/tests/Fixtures/Cqrs/FailingCommand.php b/packages/security/tests/Fixtures/Cqrs/FailingCommand.php new file mode 100644 index 00000000..f68df56a --- /dev/null +++ b/packages/security/tests/Fixtures/Cqrs/FailingCommand.php @@ -0,0 +1,11 @@ +failure); + } +} diff --git a/packages/security/tests/Fixtures/Cqrs/FailingQuery.php b/packages/security/tests/Fixtures/Cqrs/FailingQuery.php new file mode 100644 index 00000000..b057cdbc --- /dev/null +++ b/packages/security/tests/Fixtures/Cqrs/FailingQuery.php @@ -0,0 +1,11 @@ +failure); + } +} diff --git a/packages/security/tests/Fixtures/Cqrs/Failures.php b/packages/security/tests/Fixtures/Cqrs/Failures.php new file mode 100644 index 00000000..eb4d8e4a --- /dev/null +++ b/packages/security/tests/Fixtures/Cqrs/Failures.php @@ -0,0 +1,37 @@ + new RuntimeException('SELECT secret FROM users'), + 'query' => new QueryException('sqlite', self::SQL, [self::BINDING], new PDOException('SQLSTATE[HY000]: General error: 1 no such table: users')), + 'conflict' => (new ConflictException('The wallet cannot be overdrawn.', 'WALLET_OVERDRAWN'))->withExtensions(['field' => 'amount']), + 'unavailable' => new FireflyException('The ledger is closed for the night; try again after 06:00.', 'LEDGER_CLOSED', 503, ErrorCategory::Infrastructure, ErrorSeverity::Error), + default => new LogicException("Unknown failure [{$failure}]."), + }; + } +} diff --git a/samples/lumen/tests/Web/WalletRestTest.php b/samples/lumen/tests/Web/WalletRestTest.php index 874be7c9..2e793aae 100644 --- a/samples/lumen/tests/Web/WalletRestTest.php +++ b/samples/lumen/tests/Web/WalletRestTest.php @@ -180,11 +180,14 @@ // Authorized but over-balance: the domain's no-overdraw invariant raises ConflictException (409), wrapped by // the bus into CommandProcessingException, and rendered as 409 problem-details — a business fault, not a 403. + // The detail is the domain's own sentence, word for word: the bus's "Processing command [...] failed:" prefix + // stays on the wrapper's message, in the log. $this->postJson("/api/v1/wallets/{$id}/withdraw", ['amount_minor' => 999999]) ->assertStatus(409) ->assertHeader('Content-Type', 'application/problem+json') ->assertJsonPath('status', 409) - ->assertJsonPath('category', 'business'); + ->assertJsonPath('category', 'business') + ->assertJsonPath('detail', 'cannot withdraw 9999.99 EUR; balance is 20.00 EUR'); // The rejected overdraw never touched the balance. $this->getJson("/api/v1/wallets/{$id}/balance")->assertJson(['balance_minor' => 2000]); From 0cdf2e4e4a2088637c001b31eb830ced28929bd4 Mon Sep 17 00:00:00 2001 From: Andres Contreras Date: Tue, 13 Oct 2026 16:49:58 -0700 Subject: [PATCH 4/5] fix(eventsourcing,orchestration): withhold raw cause messages from the wire EventMapper's EventSerializationException for an event constructor that threw and ConditionEvaluator's ORCHESTRATION_CONDITION_FAILED quote an arbitrary throwable; both now withhold their detail. The orchestration REST guard publishes detail() and a fixed sentence when it is withheld, instead of any refusal's message. --- .../eventsourcing/src/Event/EventMapper.php | 5 +- .../tests/Event/EventMapperTest.php | 18 ++++ .../tests/Fixtures/Guarded/LimitRaised.php | 24 ++++++ .../src/Core/Condition/ConditionEvaluator.php | 5 +- .../src/Http/OrchestrationController.php | 7 +- .../tests/Core/ConditionEvaluatorTest.php | 14 ++++ .../tests/Http/OrchestrationRestTest.php | 14 ++++ .../tests/Support/WithholdingPersistence.php | 84 +++++++++++++++++++ 8 files changed, 168 insertions(+), 3 deletions(-) create mode 100644 packages/eventsourcing/tests/Fixtures/Guarded/LimitRaised.php create mode 100644 packages/orchestration/tests/Support/WithholdingPersistence.php diff --git a/packages/eventsourcing/src/Event/EventMapper.php b/packages/eventsourcing/src/Event/EventMapper.php index f700231f..29d6057a 100644 --- a/packages/eventsourcing/src/Event/EventMapper.php +++ b/packages/eventsourcing/src/Event/EventMapper.php @@ -91,7 +91,10 @@ public function toDomainEvent(StoredEvent $event): DomainEvent } catch (EventSerializationException $failure) { throw $failure; } catch (Throwable $failure) { - throw new EventSerializationException("The stored event [{$event->eventType}] could not be built as {$class}: {$failure->getMessage()}", 'EVENT_SERIALIZATION', 500, $failure); + // The message names the event class and quotes whatever the constructor threw — the log's line. A + // TypeError or an application assertion was never written for a client, so nothing is published. + throw (new EventSerializationException("The stored event [{$event->eventType}] could not be built as {$class}: {$failure->getMessage()}", 'EVENT_SERIALIZATION', 500, $failure)) + ->withDetail(null); } } diff --git a/packages/eventsourcing/tests/Event/EventMapperTest.php b/packages/eventsourcing/tests/Event/EventMapperTest.php index 805ab4b0..dfdce2a9 100644 --- a/packages/eventsourcing/tests/Event/EventMapperTest.php +++ b/packages/eventsourcing/tests/Event/EventMapperTest.php @@ -13,6 +13,7 @@ use Firefly\EventSourcing\Tests\Fixtures\Events\OrderPaid; use Firefly\EventSourcing\Tests\Fixtures\Events\OrderPlaced; use Firefly\EventSourcing\Tests\Fixtures\Events\Untyped; +use Firefly\EventSourcing\Tests\Fixtures\Guarded\LimitRaised; use Firefly\EventSourcing\Tests\Fixtures\Moods\Mood; use Firefly\EventSourcing\Tests\Fixtures\Moods\MoodChanged; @@ -63,3 +64,20 @@ function storedAs(string $type, int $version, array $payload): StoredEvent ->and($mapper->toDomainEvent(storedAs('mood.changed', 1, ['mood' => 'Grumpy', 'previous' => null])))->toEqual(new MoodChanged(Mood::Grumpy, null, 'evt-1', $stored->occurredAt)) ->and(fn () => $mapper->toDomainEvent(storedAs('mood.changed', 1, ['mood' => 'Sleepy'])))->toThrow(EventSerializationException::class, 'mood.changed.mood'); }); + +it('quotes a failing constructor in the message for the log and publishes nothing of it', function (): void { + $mapper = new EventMapper(new EventTypeRegistry((new EventSourcingScanner)->scan(['Firefly\\EventSourcing\\Tests\\Fixtures\\Guarded\\' => dirname(__DIR__).'/Fixtures/Guarded'])->eventTypes())); + + $failure = null; + try { + $mapper->toDomainEvent(storedAs('limit.raised', 1, ['limit' => -1])); + } catch (EventSerializationException $caught) { + $failure = $caught; + } + + expect($failure)->toBeInstanceOf(EventSerializationException::class) + ->and($failure?->getMessage())->toContain('secret-tenant-7') + ->and($failure?->getMessage())->toContain(LimitRaised::class) + ->and($failure?->detail())->toBeNull() + ->and($failure?->getPrevious())->toBeInstanceOf(InvalidArgumentException::class); +}); diff --git a/packages/eventsourcing/tests/Fixtures/Guarded/LimitRaised.php b/packages/eventsourcing/tests/Fixtures/Guarded/LimitRaised.php new file mode 100644 index 00000000..28e616bf --- /dev/null +++ b/packages/eventsourcing/tests/Fixtures/Guarded/LimitRaised.php @@ -0,0 +1,24 @@ +language->evaluate($expression, $values); } catch (Throwable $error) { - throw new OrchestrationException("Condition [{$expression}] could not be evaluated: {$error->getMessage()}", 'ORCHESTRATION_CONDITION_FAILED', 500, $error); + // Whatever the expression's evaluation threw is quoted for the log; it was never written for a + // client, so nothing is published (the web layer answers with its opaque sentence). + throw (new OrchestrationException("Condition [{$expression}] could not be evaluated: {$error->getMessage()}", 'ORCHESTRATION_CONDITION_FAILED', 500, $error)) + ->withDetail(null); } } } diff --git a/packages/orchestration/src/Http/OrchestrationController.php b/packages/orchestration/src/Http/OrchestrationController.php index 1ccd8bd5..9ec7e467 100644 --- a/packages/orchestration/src/Http/OrchestrationController.php +++ b/packages/orchestration/src/Http/OrchestrationController.php @@ -34,6 +34,9 @@ */ final class OrchestrationController { + /** What a refusal with no sentence for the caller answers with; the log has the cause. */ + public const string WITHHELD = 'An unexpected error occurred.'; + public function __construct( private readonly Orchestrator $orchestrator, private readonly ExecutionPersistenceProvider $persistence, @@ -160,7 +163,9 @@ private function guard(Closure $action): HttpResponse try { return $action(); } catch (FireflyException $refused) { - return new JsonResponse(['error' => $refused->getMessage(), 'code' => $refused->errorCode()], $refused->httpStatus()); + // detail(), not getMessage(): a refusal that wraps a raw cause withholds its sentence (null), and its + // message — the cause quoted verbatim — stays in the log rather than in this response. + return new JsonResponse(['error' => $refused->detail() ?? self::WITHHELD, 'code' => $refused->errorCode()], $refused->httpStatus()); } } diff --git a/packages/orchestration/tests/Core/ConditionEvaluatorTest.php b/packages/orchestration/tests/Core/ConditionEvaluatorTest.php index 8a1888bd..4a2cd436 100644 --- a/packages/orchestration/tests/Core/ConditionEvaluatorTest.php +++ b/packages/orchestration/tests/Core/ConditionEvaluatorTest.php @@ -22,3 +22,17 @@ it('wraps a runtime evaluation error', function (): void { (new ConditionEvaluator)->evaluate('input["a"]["b"] > 1', ['input' => ['a' => 3], 'results' => [], 'variables' => [], 'headers' => []]); })->throws(OrchestrationException::class, 'could not be evaluated'); + +it('keeps the evaluation error for the log and publishes nothing of it', function (): void { + $failure = null; + try { + (new ConditionEvaluator)->evaluate('input["a"]["b"] > 1', ['input' => ['a' => 3], 'results' => [], 'variables' => [], 'headers' => []]); + } catch (OrchestrationException $caught) { + $failure = $caught; + } + + expect($failure)->toBeInstanceOf(OrchestrationException::class) + ->and($failure?->getMessage())->toContain('could not be evaluated: ') + ->and($failure?->detail())->toBeNull() + ->and($failure?->getPrevious())->not->toBeNull(); +}); diff --git a/packages/orchestration/tests/Http/OrchestrationRestTest.php b/packages/orchestration/tests/Http/OrchestrationRestTest.php index 77856443..b4fc46d2 100644 --- a/packages/orchestration/tests/Http/OrchestrationRestTest.php +++ b/packages/orchestration/tests/Http/OrchestrationRestTest.php @@ -2,8 +2,11 @@ declare(strict_types=1); +use Firefly\Orchestration\Http\OrchestrationController; +use Firefly\Orchestration\Persistence\ExecutionPersistenceProvider; use Firefly\Orchestration\Tests\CapstoneFixtures\Ledger; use Firefly\Orchestration\Tests\Support\RestCapstoneTestCase; +use Firefly\Orchestration\Tests\Support\WithholdingPersistence; use Illuminate\Testing\TestResponse; use Symfony\Component\HttpFoundation\Response; @@ -89,3 +92,14 @@ function restId(TestResponse $response, string $path): string $this->postJson('/ops/orchestration/sagas/checkout', ['input' => 'nope'])->assertStatus(400)->assertJson(['code' => 'ORCHESTRATION_BAD_REQUEST']); $this->postJson('/ops/orchestration/sagas/nope', ['input' => []])->assertNotFound(); }); + +it('answers a refusal that withholds its sentence without the raw cause its message quotes', function (): void { + /** @var RestCapstoneTestCase $this */ + $this->app()->instance(ExecutionPersistenceProvider::class, new WithholdingPersistence); + + $response = $this->getJson('/ops/orchestration/executions/e-1') + ->assertStatus(500) + ->assertExactJson(['error' => OrchestrationController::WITHHELD, 'code' => 'ORCHESTRATION_STORE_FAILED']); + + expect((string) $response->getContent())->not->toContain('10.0.0.7'); +}); diff --git a/packages/orchestration/tests/Support/WithholdingPersistence.php b/packages/orchestration/tests/Support/WithholdingPersistence.php new file mode 100644 index 00000000..3f8cd0fb --- /dev/null +++ b/packages/orchestration/tests/Support/WithholdingPersistence.php @@ -0,0 +1,84 @@ +getMessage()}", 'ORCHESTRATION_STORE_FAILED', 500, $cause)) + ->withDetail(null); + } + + public function find(ExecutionCriteria $criteria): array + { + return []; + } + + public function findInFlight(int $limit = 100): array + { + return []; + } + + public function findStale(DateTimeImmutable $before, int $limit = 100): array + { + return []; + } + + public function findDue(DateTimeImmutable $now, int $limit = 100): array + { + return []; + } + + public function findBySearchAttributes(array $criteria, int $limit = 100): array + { + return []; + } + + public function updateStatus(string $correlationId, ExecutionStatus $status, DateTimeImmutable $now): void {} + + public function delete(string $correlationId): bool + { + return false; + } + + public function cleanup(DateTimeImmutable $before): int + { + return 0; + } + + public function countInFlight(): int + { + return 0; + } + + public function countByStatus(): array + { + return []; + } + + public function isHealthy(): bool + { + return true; + } +} From d2c80ca02ab5e61d85d385f7d2817cd41431f261 Mon Sep 17 00:00:00 2001 From: Andres Contreras Date: Tue, 13 Oct 2026 16:49:58 -0700 Subject: [PATCH 5/5] docs: document the published detail over the CQRS bus The wire detail is the cause's own sentence, a non-Firefly cause is opaque unless disclosed, and the "Processing command [...] failed:" prefix stays on the wrapper's message for the log. Adds the CHANGELOG Security and Changed entries. --- CHANGELOG.md | 22 ++++++++++++++++++++++ book/src-es/04-first-http-api.md | 11 ++++++----- book/src-es/07-cqrs.md | 5 ++++- book/src-es/10-security.md | 5 +++-- book/src/04-first-http-api.md | 11 ++++++----- book/src/07-cqrs.md | 5 ++++- book/src/10-security.md | 5 +++-- docs/modules/cqrs.md | 15 +++++++++++++-- docs/modules/error-handling.md | 21 +++++++++++++++++++-- docs/modules/security.md | 5 +++-- 10 files changed, 83 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0594c70f..32814911 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,28 @@ All notable changes to LaraFly are documented here. This project uses CalVer (`Y ## [Unreleased] +### Changed + +- **Over the CQRS bus, `detail` is the cause's own sentence.** A refusal raised inside a handler — a `409` from the + domain, a `#[PreAuthorize]` denial — now publishes the same problem document it publishes thrown from a + controller, `detail` included, instead of prefixing it with `Processing command [] failed:`; the HTML error + page's lede follows. The wrapper's `getMessage()` keeps the prefix for the log. The seam is the kernel's: + `FireflyException::detail()` (`getMessage()` by default) is what `ErrorResponse::fromException()` publishes, and + `withDetail(?string)` replaces it, with `null` meaning the exception holds no sentence written for a client. + +### Security + +- **A command or query handler's unhandled throwable no longer reaches the wire.** `CommandProcessingException` and + `QueryProcessingException` are `FireflyException`s whose message is `Processing command [] failed: `, and the web layer published that message as `detail` — so a handler's `QueryException` put its + SQL and bindings, and the command's class name, into a 500 problem document with `firefly.web.problem.disclose` + off, where the same throwable thrown from a controller answered with the opaque sentence. A wrapper of a cause that + is not a `FireflyException` now answers exactly like that controller: `An unexpected error occurred. It has been + logged; quote reference if you report it.` with disclosure off, and the cause's own message (no prefix) + with it on, keeping its own `COMMAND_PROCESSING_ERROR`/`QUERY_PROCESSING_ERROR` code. The same rule now covers the + event-sourcing mapper's `EventSerializationException` for an event constructor that threw, the orchestration + engine's `ORCHESTRATION_CONDITION_FAILED`, and the orchestration REST surface, which published any refusal's message. + ## [26.10.2] - 2026-10-13 ### Added diff --git a/book/src-es/04-first-http-api.md b/book/src-es/04-first-http-api.md index 8dfd2871..0f332347 100644 --- a/book/src-es/04-first-http-api.md +++ b/book/src-es/04-first-http-api.md @@ -480,6 +480,7 @@ Lo que `render()` deliberadamente **no** decide es en qué `FireflyException` se public static function toFireflyException(Throwable $e, bool $disclose = true, ?string $reference = null): FireflyException { return match (true) { + $e instanceof FireflyException && $e->detail() === null => self::withheld($e, $disclose, $reference), $e instanceof FireflyException => $e, self::isExecutionTimeLimit($e) => self::executionTimeExceeded($e, $reference), $e instanceof MethodNotAllowedHttpException => self::methodNotAllowed($e), @@ -503,9 +504,9 @@ Lo que `render()` deliberadamente **no** decide es en qué `FireflyException` se } ``` -Cinco brazos que cubren cuatro casos — el `405` tiene un brazo propio solo para que los verbos que permite puedan sacarse de la frase del enrutador y llevarse a un miembro `allowed` y a la cabecera `Allow` — y todos ellos terminan de vuelta en la misma llamada `ErrorResponse::fromException(...)->toArray()`, así que la forma de la carga útil nunca depende de qué brazo la produjo. +Seis brazos que cubren cinco casos — el `405` tiene un brazo propio solo para que los verbos que permite puedan sacarse de la frase del enrutador y llevarse a un miembro `allowed` y a la cabecera `Allow` — y todos ellos terminan de vuelta en la misma llamada `ErrorResponse::fromException(...)->toArray()`, así que la forma de la carga útil nunca depende de qué brazo la produjo. -Una `FireflyException` — o una de sus subclases tipadas, como `ResourceNotFoundException` — se devuelve intacta y se renderiza con su propio `httpStatus()`, porque su mensaje lo escribió tu aplicación *para* el cliente; ese es justamente el sentido de la taxonomía. El propio límite de tiempo de ejecución de PHP se nombra aparte y responde `503` con una cabecera `Retry-After`, porque «el servidor detuvo esta petición a los N segundos» es algo sobre lo que quien llama puede actuar y un `500` desnudo no lo es. Una excepción HTTP de Laravel/Symfony — una URL que no coincide con ninguna ruta, un verbo que una ruta no acepta — conserva su **código de estado real**, así que una ruta no coincidente sigue respondiendo `404` y nunca un `500` engañoso; solo se reemplaza la redacción del propio enrutador por una frase escrita para una persona, y los verbos permitidos de un `405` pasan a un miembro `allowed` y a la cabecera `Allow`, donde un cliente puede leerlos sin analizar inglés. Cualquier otra cosa es un accidente, y `$disclose` — `firefly.web.problem.disclose`, por defecto `false` — decide si su mensaje puede publicarse siquiera: con él apagado el cuerpo lleva una frase fija que nombra la misma referencia que lleva el `traceId` del documento, y el mensaje real se queda en el log, que es donde el SQL de una `QueryException` y sus enlaces deben estar. +Una `FireflyException` — o una de sus subclases tipadas, como `ResourceNotFoundException` — se devuelve intacta y se renderiza con su propio `httpStatus()`, porque su mensaje lo escribió tu aplicación *para* el cliente; ese es justamente el sentido de la taxonomía. Lo que llega al cable es el `detail()` de la excepción, que es ese mensaje salvo que la excepción diga otra cosa — y el primer brazo es para la que lo dice: una `FireflyException` cuyo `detail()` es `null` no lleva ninguna frase para el cliente (el envoltorio del bus CQRS sobre una `QueryException` es una, Capítulo 7), así que conserva su propio código, estado, categoría y miembros de extensión, pero toma su frase exactamente como el último brazo se la daría al throwable que envuelve. El propio límite de tiempo de ejecución de PHP se nombra aparte y responde `503` con una cabecera `Retry-After`, porque «el servidor detuvo esta petición a los N segundos» es algo sobre lo que quien llama puede actuar y un `500` desnudo no lo es. Una excepción HTTP de Laravel/Symfony — una URL que no coincide con ninguna ruta, un verbo que una ruta no acepta — conserva su **código de estado real**, así que una ruta no coincidente sigue respondiendo `404` y nunca un `500` engañoso; solo se reemplaza la redacción del propio enrutador por una frase escrita para una persona, y los verbos permitidos de un `405` pasan a un miembro `allowed` y a la cabecera `Allow`, donde un cliente puede leerlos sin analizar inglés. Cualquier otra cosa es un accidente, y `$disclose` — `firefly.web.problem.disclose`, por defecto `false` — decide si su mensaje puede publicarse siquiera: con él apagado el cuerpo lleva una frase fija que nombra la misma referencia que lleva el `traceId` del documento, y el mensaje real se queda en el log, que es donde el SQL de una `QueryException` y sus enlaces deben estar. Pedir un monedero que nunca se abrió se renderiza así. Fíjate en `instance`: el RFC 9457 §3.1.5 lo define como una **referencia** URI, y una referencia relativa se resuelve contra la URI base del documento — así que el `api/v1/wallets/wlt-999` pelado que devuelve `$request->path()`, servido desde `/api/v1/wallets/wlt-999`, identificaría `/api/v1/api/v1/wallets/wlt-999`. LaraFly publica la forma relativa a la raíz, que es el único trabajo de `ProblemMapper::instanceFor()`, y un cliente puede compararla con la ruta que pidió. Fíjate también en `type`: el RFC 9457 §3.1.1 dice que un `type` ausente *es* `about:blank`, y LaraFly lo escribe en vez de dejar que el lector tenga que saberlo — apunta `firefly.web.problem.type-uri` a una URI base y el `code` estable deriva uno real y abrible. @@ -557,7 +558,7 @@ Un intento de retiro se rechaza por dos vías distintas, y las dos **no dan el m "code": "AUTHENTICATION_FAILED", "category": "security", "severity": "warning", - "detail": "Processing command [Lumen\\Application\\Command\\Withdraw] failed: Authentication is required.", + "detail": "Authentication is required.", "type": "about:blank", "instance": "/api/v1/wallets/wlt-1/withdraw", "traceId": "4bf92f3577b34da6a3ce929d0e0e4736", @@ -575,7 +576,7 @@ El `403` queda reservado para un principal que **sí** ha iniciado sesión y aun "code": "ACCESS_DENIED", "category": "security", "severity": "warning", - "detail": "Processing command [Lumen\\Application\\Command\\Withdraw] failed: You do not have permission to do this.", + "detail": "You do not have permission to do this.", "type": "about:blank", "instance": "/api/v1/wallets/wlt-1/withdraw", "traceId": "4bf92f3577b34da6a3ce929d0e0e4736", @@ -585,7 +586,7 @@ El `403` queda reservado para un principal que **sí** ha iniciado sesión y aun } ``` -Los dos se producen sin ningún cambio de código en `WalletController::withdraw()`: el bus envuelve la excepción de seguridad que haya levantado el evaluador en una `CommandProcessingException` que copia el código, el estado, la categoría y los miembros de extensión de la causa, así que el cable lleva la identidad de la causa y no un error genérico del bus, y el 403 nombra las mismas `requiredAuthorities` que nombraría un método de controlador rechazado. Solo `detail` delata que pasó por el bus: el envoltorio antepone a la frase de la causa el comando que estaba procesando. El Capítulo 10 vuelve sobre este par — y sobre las dos pruebas distribuidas que lo fijan — en detalle. +Los dos se producen sin ningún cambio de código en `WalletController::withdraw()`: el bus envuelve la excepción de seguridad que haya levantado el evaluador en una `CommandProcessingException` que copia el código, el estado, la categoría y los miembros de extensión de la causa, así que el cable lleva la identidad de la causa y no un error genérico del bus, y el 403 nombra las mismas `requiredAuthorities` que nombraría un método de controlador rechazado. Ni siquiera `detail` delata que pasó por el bus: es la propia frase de la causa, palabra por palabra. El *mensaje* del envoltorio sigue nombrando el comando que estaba procesando — `Processing command [Lumen\Application\Command\Withdraw] failed: …` — pero esa línea es para el log, y el Capítulo 7 explica por qué se mantienen separados. El Capítulo 10 vuelve sobre este par — y sobre las dos pruebas distribuidas que lo fijan — en detalle. ### Manejar una excepción tú mismo diff --git a/book/src-es/07-cqrs.md b/book/src-es/07-cqrs.md index 7a34672a..9b9733d4 100644 --- a/book/src-es/07-cqrs.md +++ b/book/src-es/07-cqrs.md @@ -414,11 +414,14 @@ final class CommandProcessingException extends CqrsException $cause instanceof FireflyException ? $cause->extensions() : [], $cause instanceof FireflyException ? $cause->title() : null, ); + + // … + $this->withDetail($cause instanceof FireflyException ? $cause->detail() : null); } } ``` -Tres cosas merecen destacarse. Primero, la causa siempre está disponible en `getPrevious()` — el quinto argumento del constructor hacia el padre — así que nada del fallo original se pierde, solo se envuelve. Segundo, esto **preserva la categoría**: cuando la causa es una `FireflyException` (todas las excepciones del kernel lo son), su propio `httpStatus()`/`category()`/`severity()` se copian sobre el envoltorio en vez de recaer en un genérico `500`/`Internal`, y también sus miembros de extensión RFC 9457 y su título, razón por la cual un `#[PreAuthorize]` rechazado sigue nombrando sus `requiredAuthorities` en el documento de problema después de que el bus lo envuelva (el Capítulo 4 muestra ese documento). Una `ResourceNotFoundException` (404, negocio) lanzada por un id de monedero desconocido sigue renderizándose como una respuesta de detalles de problema 404 incluso después de ser envuelta — el bus añade contexto, nunca enmascara el fallo. Tercero, `send()` comprueba `$e instanceof CommandProcessingException` antes de envolver, así que un despacho de comando anidado (un manejador que a su vez llama a `$commands->send(...)`) nunca queda envuelto dos veces en dos capas de la misma excepción. `QueryProcessingException` es el gemelo del lado de lectura, idéntico en forma. +Cuatro cosas merecen destacarse. Primero, la causa siempre está disponible en `getPrevious()` — el quinto argumento del constructor hacia el padre — así que nada del fallo original se pierde, solo se envuelve. Segundo, esto **preserva la categoría**: cuando la causa es una `FireflyException` (todas las excepciones del kernel lo son), su propio `httpStatus()`/`category()`/`severity()` se copian sobre el envoltorio en vez de recaer en un genérico `500`/`Internal`, y también sus miembros de extensión RFC 9457 y su título, razón por la cual un `#[PreAuthorize]` rechazado sigue nombrando sus `requiredAuthorities` en el documento de problema después de que el bus lo envuelva (el Capítulo 4 muestra ese documento). Una `ResourceNotFoundException` (404, negocio) lanzada por un id de monedero desconocido sigue renderizándose como una respuesta de detalles de problema 404 incluso después de ser envuelta — el bus añade contexto, nunca enmascara el fallo. Tercero, `send()` comprueba `$e instanceof CommandProcessingException` antes de envolver, así que un despacho de comando anidado (un manejador que a su vez llama a `$commands->send(...)`) nunca queda envuelto dos veces en dos capas de la misma excepción. `QueryProcessingException` es el gemelo del lado de lectura, idéntico en forma. Cuarto, el mensaje y el `detail` son **dos frases distintas**. El mensaje — `Processing command [Lumen\Application\Command\Withdraw] failed: …` — nombra el comando y cita la causa al pie de la letra, que es lo que quiere una línea de log. El documento de problema publica en su lugar `detail()`, y `withDetail()` lo fija al de la causa: una causa redactada llega al cliente palabra por palabra, exactamente como si la hubiera lanzado un controlador, y una causa que *no* es una `FireflyException` — una `QueryException` con su SQL y sus parámetros, un `TypeError` que nombra una ruta del servidor — no tiene ninguna frase para el cliente, así que el detalle es `null` y la capa web responde con la misma frase opaca que da a ese mismo throwable lanzado desde un controlador (o, con `firefly.web.problem.disclose=true`, el propio mensaje de la causa sin el prefijo). El envoltorio conserva su propio código `500 COMMAND_PROCESSING_ERROR`; solo se retiene la frase. `HandlerRegistry` lanza sus propias excepciones, más estrechas, para un error de **cableado** en vez de un fallo de negocio — un mensaje sin mapear enviado a través del bus: diff --git a/book/src-es/10-security.md b/book/src-es/10-security.md index 7b4fb7a3..54ca6f8e 100644 --- a/book/src-es/10-security.md +++ b/book/src-es/10-security.md @@ -1165,10 +1165,11 @@ it('allows an authorized withdraw and renders an overdraw as 409 problem+json', $this->postJson("/api/v1/wallets/{$id}/withdraw", ['amount_minor' => 999999]) ->assertStatus(409) // … - ->assertJsonPath('category', 'business'); + ->assertJsonPath('category', 'business') + ->assertJsonPath('detail', 'cannot withdraw 9999.99 EUR; balance is 20.00 EUR'); ``` -La autoridad concedida es `'ROLE_WALLET_OWNER'` — ya con el prefijo `ROLE_` incorporado — y la expresión de `WithdrawHandler` llama a `hasRole('WALLET_OWNER')`, que `SecurityExpressionRoot::hasRole()` normaliza a la cadena idéntica `'ROLE_WALLET_OWNER'` antes de comprobar la pertenencia; las dos formas de escribirlo se encuentran en el medio. El principal es `owner-4` y el monedero se abre para `owner-4`, y eso es lo que permite que `hasPermission(#command, 'debit')` conceda el cargo. El `409` de la segunda llamada es el contraste importante: la autorización y la validación de dominio son dos capas genuinamente independientes — que se te *permita* retirar no dice nada sobre si la retirada en sí es *válida*, y la propia invariante de no-sobregiro de `Wallet::withdraw()` (Capítulo 6) sigue aplicándose exactamente igual que antes. +La autoridad concedida es `'ROLE_WALLET_OWNER'` — ya con el prefijo `ROLE_` incorporado — y la expresión de `WithdrawHandler` llama a `hasRole('WALLET_OWNER')`, que `SecurityExpressionRoot::hasRole()` normaliza a la cadena idéntica `'ROLE_WALLET_OWNER'` antes de comprobar la pertenencia; las dos formas de escribirlo se encuentran en el medio. El principal es `owner-4` y el monedero se abre para `owner-4`, y eso es lo que permite que `hasPermission(#command, 'debit')` conceda el cargo. El `409` de la segunda llamada es el contraste importante: la autorización y la validación de dominio son dos capas genuinamente independientes — que se te *permita* retirar no dice nada sobre si la retirada en sí es *válida*, y la propia invariante de no-sobregiro de `Wallet::withdraw()` (Capítulo 6) sigue aplicándose exactamente igual que antes. Su `detail` es la frase del dominio palabra por palabra: el envoltorio del bus guarda la clase del comando para el log y publica la frase propia de la causa. `samples/lumen/tests/Application/TransferSecurityTest.php` demuestra el mismo guardián directamente contra el `CommandBus`, con el patrón de desenvolver mediante `getPrevious()` que introdujo el Capítulo 7: diff --git a/book/src/04-first-http-api.md b/book/src/04-first-http-api.md index 46aeb907..e4e0b021 100644 --- a/book/src/04-first-http-api.md +++ b/book/src/04-first-http-api.md @@ -480,6 +480,7 @@ What `render()` deliberately does **not** decide is which `FireflyException` an public static function toFireflyException(Throwable $e, bool $disclose = true, ?string $reference = null): FireflyException { return match (true) { + $e instanceof FireflyException && $e->detail() === null => self::withheld($e, $disclose, $reference), $e instanceof FireflyException => $e, self::isExecutionTimeLimit($e) => self::executionTimeExceeded($e, $reference), $e instanceof MethodNotAllowedHttpException => self::methodNotAllowed($e), @@ -503,9 +504,9 @@ What `render()` deliberately does **not** decide is which `FireflyException` an } ``` -Five arms covering four cases — the `405` has an arm of its own only so the verbs it permits can be lifted out of the router's sentence and into an `allowed` member and the `Allow` header — and every one of them ends back at the same `ErrorResponse::fromException(...)->toArray()` call, so the payload's shape never depends on which arm produced it. +Six arms covering five cases — the `405` has an arm of its own only so the verbs it permits can be lifted out of the router's sentence and into an `allowed` member and the `Allow` header — and every one of them ends back at the same `ErrorResponse::fromException(...)->toArray()` call, so the payload's shape never depends on which arm produced it. -A `FireflyException` — or one of its typed subclasses, like `ResourceNotFoundException` — is returned untouched and renders at its own `httpStatus()`, because its message was written by your application *for* the client; that is the whole point of the taxonomy. PHP's own execution-time limit is named separately and answers `503` with a `Retry-After` header, because "the server stopped this request after N seconds" is something a caller can act on and a bare `500` is not. A Laravel/Symfony HTTP exception — a URL matching no route, a verb a route does not accept — keeps its **real status code**, so an unmatched route still answers `404` and never a misleading `500`; only the router's own wording is replaced with a sentence written for a person, and a `405`'s permitted verbs move into an `allowed` member and the `Allow` header where a client can read them without parsing English. Anything else is an accident, and `$disclose` — `firefly.web.problem.disclose`, default `false` — decides whether its message may be published at all: with it off the body carries a fixed sentence naming the same reference the document's `traceId` carries, and the real message stays in the log, which is where a `QueryException`'s SQL and its bindings belong. +A `FireflyException` — or one of its typed subclasses, like `ResourceNotFoundException` — is returned untouched and renders at its own `httpStatus()`, because its message was written by your application *for* the client; that is the whole point of the taxonomy. What reaches the wire is the exception's `detail()`, which is that message unless the exception said otherwise — and the first arm is for the one that did: a `FireflyException` whose `detail()` is `null` holds no sentence for a client (the CQRS bus's wrapper of a `QueryException` is one, Chapter 7), so it keeps its own code, status, category and extension members but takes its sentence exactly as the last arm would give it to the throwable it wraps. PHP's own execution-time limit is named separately and answers `503` with a `Retry-After` header, because "the server stopped this request after N seconds" is something a caller can act on and a bare `500` is not. A Laravel/Symfony HTTP exception — a URL matching no route, a verb a route does not accept — keeps its **real status code**, so an unmatched route still answers `404` and never a misleading `500`; only the router's own wording is replaced with a sentence written for a person, and a `405`'s permitted verbs move into an `allowed` member and the `Allow` header where a client can read them without parsing English. Anything else is an accident, and `$disclose` — `firefly.web.problem.disclose`, default `false` — decides whether its message may be published at all: with it off the body carries a fixed sentence naming the same reference the document's `traceId` carries, and the real message stays in the log, which is where a `QueryException`'s SQL and its bindings belong. Requesting a wallet that was never opened renders like this. Note `instance`: RFC 9457 §3.1.5 makes it a URI **reference**, and a relative reference resolves against the document's base URI — so the bare `api/v1/wallets/wlt-999` that `$request->path()` answers, served from `/api/v1/wallets/wlt-999`, would identify `/api/v1/api/v1/wallets/wlt-999`. LaraFly publishes the root-relative form, `ProblemMapper::instanceFor()`'s one job, and a client may compare it to the path it asked for. Note `type` too: RFC 9457 §3.1.1 says an absent `type` *is* `about:blank`, and LaraFly writes it out rather than leaving the reader to know that — point `firefly.web.problem.type-uri` at a base URI instead and the stable `code` derives a real, openable one. @@ -557,7 +558,7 @@ A withdraw attempt is refused twice over, and the two refusals are **not the sam "code": "AUTHENTICATION_FAILED", "category": "security", "severity": "warning", - "detail": "Processing command [Lumen\\Application\\Command\\Withdraw] failed: Authentication is required.", + "detail": "Authentication is required.", "type": "about:blank", "instance": "/api/v1/wallets/wlt-1/withdraw", "traceId": "4bf92f3577b34da6a3ce929d0e0e4736", @@ -575,7 +576,7 @@ A withdraw attempt is refused twice over, and the two refusals are **not the sam "code": "ACCESS_DENIED", "category": "security", "severity": "warning", - "detail": "Processing command [Lumen\\Application\\Command\\Withdraw] failed: You do not have permission to do this.", + "detail": "You do not have permission to do this.", "type": "about:blank", "instance": "/api/v1/wallets/wlt-1/withdraw", "traceId": "4bf92f3577b34da6a3ce929d0e0e4736", @@ -585,7 +586,7 @@ A withdraw attempt is refused twice over, and the two refusals are **not the sam } ``` -Both are produced with no code change to `WalletController::withdraw()` at all: the bus wraps whichever security exception the evaluator raised in a `CommandProcessingException` that copies the cause's code, status, category and extension members, so the wire carries the cause's identity rather than a generic bus error, and the 403 names the same `requiredAuthorities` a refused controller method would. Only `detail` says it came through the bus: the wrapper prefixes the cause's sentence with the command it was processing. Chapter 10 returns to this pair — and to the two shipped tests that pin it — in full. +Both are produced with no code change to `WalletController::withdraw()` at all: the bus wraps whichever security exception the evaluator raised in a `CommandProcessingException` that copies the cause's code, status, category and extension members, so the wire carries the cause's identity rather than a generic bus error, and the 403 names the same `requiredAuthorities` a refused controller method would. Not even `detail` says it came through the bus: it is the cause's own sentence, word for word. The wrapper's *message* still names the command it was processing — `Processing command [Lumen\Application\Command\Withdraw] failed: …` — but that line is for the log, and Chapter 7 shows why the two are kept apart. Chapter 10 returns to this pair — and to the two shipped tests that pin it — in full. ### Handling an exception yourself diff --git a/book/src/07-cqrs.md b/book/src/07-cqrs.md index 3d8f8bf7..c66255c8 100644 --- a/book/src/07-cqrs.md +++ b/book/src/07-cqrs.md @@ -414,11 +414,14 @@ final class CommandProcessingException extends CqrsException $cause instanceof FireflyException ? $cause->extensions() : [], $cause instanceof FireflyException ? $cause->title() : null, ); + + // … + $this->withDetail($cause instanceof FireflyException ? $cause->detail() : null); } } ``` -Three things to notice. First, the cause is always available on `getPrevious()` — the constructor's fifth argument to the parent — so nothing about the original failure is lost, only wrapped. Second, this is **category-preserving**: when the cause is a `FireflyException` (every kernel exception is), its own `httpStatus()`/`category()`/`severity()` are copied onto the wrapper rather than defaulting to a generic `500`/`Internal`, and so are its RFC 9457 extension members and title, which is why a refused `#[PreAuthorize]` still names its `requiredAuthorities` in the problem document after the bus has wrapped it (Chapter 4 shows that document). A `ResourceNotFoundException` (404, business) thrown for an unknown wallet id still renders as a 404 problem-details response even after being wrapped — the bus adds context, it never masks the fault. Third, `send()` checks `$e instanceof CommandProcessingException` before wrapping, so a nested command dispatch (a handler that itself calls `$commands->send(...)`) never gets double-wrapped in two layers of the same exception. `QueryProcessingException` is the read-side twin, identical in shape. +Four things to notice. First, the cause is always available on `getPrevious()` — the constructor's fifth argument to the parent — so nothing about the original failure is lost, only wrapped. Second, this is **category-preserving**: when the cause is a `FireflyException` (every kernel exception is), its own `httpStatus()`/`category()`/`severity()` are copied onto the wrapper rather than defaulting to a generic `500`/`Internal`, and so are its RFC 9457 extension members and title, which is why a refused `#[PreAuthorize]` still names its `requiredAuthorities` in the problem document after the bus has wrapped it (Chapter 4 shows that document). A `ResourceNotFoundException` (404, business) thrown for an unknown wallet id still renders as a 404 problem-details response even after being wrapped — the bus adds context, it never masks the fault. Third, `send()` checks `$e instanceof CommandProcessingException` before wrapping, so a nested command dispatch (a handler that itself calls `$commands->send(...)`) never gets double-wrapped in two layers of the same exception. `QueryProcessingException` is the read-side twin, identical in shape. Fourth, the message and the `detail` are **two different sentences**. The message — `Processing command [Lumen\Application\Command\Withdraw] failed: …` — names the command and quotes the cause verbatim, which is what a log line wants. The problem document publishes `detail()` instead, and `withDetail()` sets it to the cause's own: an authored cause reaches the client word for word, exactly as if a controller had thrown it, and a cause that is *not* a `FireflyException` — a `QueryException` carrying its SQL and bindings, a `TypeError` naming a path on the server — has no sentence for a client at all, so the detail is `null` and the web layer answers with the same opaque sentence it gives that throwable thrown from a controller (or, with `firefly.web.problem.disclose=true`, the cause's own message without the prefix). The wrapper still keeps its own `500 COMMAND_PROCESSING_ERROR` code; only the sentence is withheld. `HandlerRegistry` throws its own, narrower exceptions for a **wiring** mistake rather than a business fault — an unmapped message sent through the bus: diff --git a/book/src/10-security.md b/book/src/10-security.md index 1095ff82..1668d38c 100644 --- a/book/src/10-security.md +++ b/book/src/10-security.md @@ -1165,10 +1165,11 @@ it('allows an authorized withdraw and renders an overdraw as 409 problem+json', $this->postJson("/api/v1/wallets/{$id}/withdraw", ['amount_minor' => 999999]) ->assertStatus(409) // … - ->assertJsonPath('category', 'business'); + ->assertJsonPath('category', 'business') + ->assertJsonPath('detail', 'cannot withdraw 9999.99 EUR; balance is 20.00 EUR'); ``` -The granted authority is `'ROLE_WALLET_OWNER'` — already carrying the `ROLE_` prefix — and `WithdrawHandler`'s expression calls `hasRole('WALLET_OWNER')`, which `SecurityExpressionRoot::hasRole()` normalises to the identical `'ROLE_WALLET_OWNER'` string before checking membership; the two spellings meet in the middle. The principal is `owner-4` and the wallet is opened for `owner-4`, which is what lets `hasPermission(#command, 'debit')` grant the debit. The `409` in the second call is the important contrast: authorization and domain validation are two genuinely independent layers — being *allowed* to withdraw says nothing about whether the withdrawal itself is *valid*, and `Wallet::withdraw()`'s own no-overdraw invariant (Chapter 6) still applies exactly as before. +The granted authority is `'ROLE_WALLET_OWNER'` — already carrying the `ROLE_` prefix — and `WithdrawHandler`'s expression calls `hasRole('WALLET_OWNER')`, which `SecurityExpressionRoot::hasRole()` normalises to the identical `'ROLE_WALLET_OWNER'` string before checking membership; the two spellings meet in the middle. The principal is `owner-4` and the wallet is opened for `owner-4`, which is what lets `hasPermission(#command, 'debit')` grant the debit. The `409` in the second call is the important contrast: authorization and domain validation are two genuinely independent layers — being *allowed* to withdraw says nothing about whether the withdrawal itself is *valid*, and `Wallet::withdraw()`'s own no-overdraw invariant (Chapter 6) still applies exactly as before. Its `detail` is the domain's sentence word for word: the bus's wrapper keeps the command class for the log and publishes the cause's own sentence. `samples/lumen/tests/Application/TransferSecurityTest.php` proves the same guard directly against the `CommandBus`, with the `getPrevious()` unwrap pattern Chapter 7 introduced: diff --git a/docs/modules/cqrs.md b/docs/modules/cqrs.md index 850e31d1..e39b23ef 100644 --- a/docs/modules/cqrs.md +++ b/docs/modules/cqrs.md @@ -25,8 +25,19 @@ Any handler/stage throwable is re-thrown **wrapped** in a category-preserving means a `ValidationException` (422) or a not-found (500 framework) keeps its own error code / HTTP status / category / severity for RFC-7807 rendering — the wrapper never masks an expected fault to a generic 500 — and its RFC 9457 extension members and title too, so a refused `#[PreAuthorize]` publishes the same -`requiredAuthorities` through the bus as it does from a controller. Only `detail` differs: the wrapper prefixes -the cause's message with the message class it was processing. +`requiredAuthorities` through the bus as it does from a controller. + +The wrapper's **message** and its published **detail** are two different sentences. The message — +`Processing command [App\Withdraw] failed: ` — names the message class and quotes the +cause verbatim, for the log and the `previous` chain. The problem document publishes `detail()` instead, which +the wrapper sets to the cause's own: an authored `FireflyException` cause reaches the client word for word, +exactly as if a controller had thrown it. A cause that is **not** a `FireflyException` — a `QueryException` +carrying its SQL and bindings, a `TypeError` naming a server path — has no sentence for a client, so the +wrapper's detail is `null` and the web layer answers it the way it answers that throwable thrown from a +controller: the opaque `An unexpected error occurred. It has been logged; quote reference if you +report it.` with `firefly.web.problem.disclose` off (the default), and the cause's own message, without the +prefix, with it on. The wrapper keeps its own `500 COMMAND_PROCESSING_ERROR` / `QUERY_PROCESSING_ERROR` code +either way; only the sentence is withheld. ## Handlers diff --git a/docs/modules/error-handling.md b/docs/modules/error-handling.md index 01912625..cb72b8db 100644 --- a/docs/modules/error-handling.md +++ b/docs/modules/error-handling.md @@ -96,6 +96,21 @@ for a subclass that wants to fix them. An extension can never override a standar writes `status`, `title`, `code` and the rest *over* the extensions, so `['status' => 999]` is harmless. The generated OpenAPI problem schema declares `additionalProperties: true` for the same reason. +## The published sentence: `detail()` + +`ErrorResponse::fromException()` publishes `detail()` as `detail`, not `getMessage()`. By default the two are +the same, because a message in this taxonomy is written for the client. A **wrapper** is where they part: +`withDetail(?string)` replaces the published sentence on the instance — and returns it, like +`withExtensions()` — without touching the message, so the log and the `previous` chain keep every word. +`withDetail(null)` says the exception holds **no** sentence written for a client. The web layer then treats +it like an unhandled throwable for the sentence only (see the table below), while keeping its code, status, +category, severity, title and extension members. The CQRS wrappers use it: their message is +`Processing command [App\…] failed: `, and their detail is the cause's own `detail()`, or `null` when +the cause is not a `FireflyException` ([CQRS](cqrs.md)). So are the event-sourcing mapper's +`EventSerializationException` for an event constructor that threw, and the orchestration engine's +`ORCHESTRATION_CONDITION_FAILED`; the orchestration REST surface answers a withheld refusal with +`An unexpected error occurred.` + ## Field-level validation errors `ValidationException` additionally carries `Firefly\Kernel\Error\FieldError` instances — the shape of Spring's @@ -185,11 +200,13 @@ the member can never name a different origin however the request target was spel `Firefly\Web\Error\ProblemMapper` owns the rule for turning *any* throwable into that shape, in one place, because the HTML page below needs the same answer and two copies of it would eventually tell a browser and a -client different things about one failure. It has **three** cases, and only the third is a disclosure: +client different things about one failure. It has **four** cases, and the disclosure gate below decides two +of them: | Throwable | Status | Whose message is it? | |---|---|---| -| A `FireflyException` | its own `httpStatus()` | the application's, written **for** the client | +| A `FireflyException` | its own `httpStatus()` | its `detail()`: the application's, written **for** the client | +| A `FireflyException` whose `detail()` is `null` (a CQRS wrapper of a `QueryException`) | its own `httpStatus()` | nobody's: withheld exactly like the last row, from the throwable it wraps | | An `HttpExceptionInterface` (the router's own 404, `abort(409, '…')`) | its real status | the author's, via `abort()` | | Anything else | 500 `INTERNAL_ERROR` | **an accident**, and withheld — see below | diff --git a/docs/modules/security.md b/docs/modules/security.md index 740756cf..e623706a 100644 --- a/docs/modules/security.md +++ b/docs/modules/security.md @@ -192,8 +192,9 @@ to the log**, at warning, with the principal and its authorities — they used t ("Access is denied for [App\Ctrl::admin].") and a PHP class name is not something a person should read on a panel. A rule on a command or query handler refuses with the same document through the bus: the CQRS wrappers copy the cause's code, status and extension members, so `requiredAuthorities` survives the wrap, and -only `detail` differs, prefixed with `Processing command [App\…] failed:`. A rule can carry its own product -code and sentence: +the two documents are identical, `detail` included: the wrapper's message keeps +`Processing command [App\…] failed:` for the log, while the `detail()` it publishes is the cause's own +sentence. A rule can carry its own product code and sentence: ```php