Skip to content

Commit 1cd4821

Browse files
lcharettelcharette
committed
Refactor error handling
Remove AlertStream from default UserMessageExceptionHandler and implement backup AlertStreamExceptionHandler
1 parent ef31841 commit 1cd4821

File tree

8 files changed

+225
-101
lines changed

8 files changed

+225
-101
lines changed

‎packages/sprinkle-account/app/tests/Twig/AccountExtensionTest.php‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
use Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration;
1515
use PHPUnit\Framework\TestCase;
1616
use Slim\Views\Twig;
17-
use UserFrosting\Alert\AlertStream;
1817
use UserFrosting\Sprinkle\Account\Authenticate\Authenticator;
1918
use UserFrosting\Sprinkle\Account\Database\Models\Interfaces\UserInterface;
2019
use UserFrosting\Sprinkle\Account\Twig\AccountExtension;
@@ -78,7 +77,7 @@ public function testCheckAccess(): void
7877

7978
public function testCurrentUser(): void
8079
{
81-
// Define mock AlertStream and register with Container
80+
// Define mock Authenticator and register with Container
8281
/** @var Authenticator */
8382
$authenticator = Mockery::mock(Authenticator::class);
8483

‎packages/sprinkle-admin/app/tests/Controller/User/UserEditActionTest.php‎

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
namespace UserFrosting\Sprinkle\Admin\Tests\Controller\User;
1414

1515
use Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration;
16-
use UserFrosting\Alert\AlertStream;
1716
use UserFrosting\Config\Config;
1817
use UserFrosting\Sprinkle\Account\Database\Models\User;
1918
use UserFrosting\Sprinkle\Account\Testing\WithTestUser;
@@ -179,13 +178,6 @@ public function testPageForEmailInUse(): void
179178
'description' => 'Email <strong>' . $user->email . '</strong> is already in use.',
180179
'status' => 400,
181180
], $response);
182-
183-
// Test message
184-
// TODO : AlertStream should not be used anymore, but there really is a message returned here.
185-
/** @var AlertStream */
186-
$ms = $this->ci->get(AlertStream::class);
187-
$messages = $ms->getAndClearMessages();
188-
$this->assertSame('danger', array_reverse($messages)[0]['type']);
189181
}
190182

191183
public function testPageForFailedValidation(): void
@@ -215,11 +207,5 @@ public function testPageForFailedValidation(): void
215207
'description' => 'Invalid email address.',
216208
'status' => 400,
217209
], $response);
218-
219-
// Test message
220-
/** @var AlertStream */
221-
$ms = $this->ci->get(AlertStream::class);
222-
$messages = $ms->getAndClearMessages();
223-
$this->assertSame('danger', array_reverse($messages)[0]['type']);
224210
}
225211
}

‎packages/sprinkle-core/app/src/Error/Handler/AlertStreamExceptionHandler.php‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/*
6+
* UserFrosting Core Sprinkle (http://www.userfrosting.com)
7+
*
8+
* @link https://github.com/userfrosting/sprinkle-core
9+
* @copyright Copyright (c) 2013-2024 Alexander Weissman & Louis Charette
10+
* @license https://github.com/userfrosting/sprinkle-core/blob/master/LICENSE.md (MIT License)
11+
*/
12+
13+
namespace UserFrosting\Sprinkle\Core\Error\Handler;
14+
15+
use DI\Attribute\Inject;
16+
use Psr\Http\Message\ResponseInterface;
17+
use Psr\Http\Message\ServerRequestInterface;
18+
use Throwable;
19+
use UserFrosting\Alert\AlertStream;
20+
use UserFrosting\Sprinkle\Core\Exceptions\Contracts\UserMessageException;
21+
use UserFrosting\Support\Message\UserMessage;
22+
23+
/**
24+
* Handles exceptions intended to be shown to the end user via the Alert Stream
25+
* service. Overrides the default behavior and status code.
26+
*/
27+
final class AlertStreamExceptionHandler extends ExceptionHandler
28+
{
29+
#[Inject]
30+
protected AlertStream $alert;
31+
32+
/**
33+
* Don't log theses exceptions.
34+
*/
35+
protected function shouldLogExceptions(): bool
36+
{
37+
return false;
38+
}
39+
40+
/**
41+
* Don't display details for theses exceptions.
42+
*/
43+
protected function displayErrorDetails(): bool
44+
{
45+
return false;
46+
}
47+
48+
/**
49+
* Force the use if Exception code for AuthException.
50+
*/
51+
protected function determineStatusCode(ServerRequestInterface $request, Throwable $exception): int
52+
{
53+
return intval($exception->getCode());
54+
}
55+
56+
/**
57+
* {@inheritdoc}
58+
*
59+
* Adds the exception message to the alert stream.
60+
*/
61+
public function handle(ServerRequestInterface $request, Throwable $exception): ResponseInterface
62+
{
63+
if ($exception instanceof UserMessageException) {
64+
$title = $this->translateExceptionPart($exception->getTitle());
65+
$description = $this->translateExceptionPart($exception->getDescription());
66+
$this->alert->addMessage('danger', "$title: $description");
67+
}
68+
69+
return parent::handle($request, $exception);
70+
}
71+
72+
/**
73+
* Translate a string or UserMessage to a string.
74+
*
75+
* @param string|UserMessage $message
76+
*
77+
* @return string
78+
*/
79+
protected function translateExceptionPart(string|UserMessage $message): string
80+
{
81+
if ($message instanceof UserMessage) {
82+
return $this->translator->translate($message->message, $message->parameters);
83+
}
84+
85+
return $this->translator->translate($message);
86+
}
87+
}

‎packages/sprinkle-core/app/src/Error/Handler/UserMessageExceptionHandler.php‎

Lines changed: 6 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -12,23 +12,19 @@
1212

1313
namespace UserFrosting\Sprinkle\Core\Error\Handler;
1414

15-
use DI\Attribute\Inject;
16-
use Psr\Http\Message\ResponseInterface;
1715
use Psr\Http\Message\ServerRequestInterface;
1816
use Throwable;
19-
use UserFrosting\Alert\AlertStream;
20-
use UserFrosting\Sprinkle\Core\Exceptions\Contracts\UserMessageException;
21-
use UserFrosting\Support\Message\UserMessage;
2217

2318
/**
24-
* Generic handler for exceptions that will be presented to the end user.
25-
* Override the default behavior and status code.
19+
* Handles exceptions intended to be presented to the end user. Overrides the
20+
* default behavior and status code. Exceptions handled by this class are not
21+
* logged and do not display detailed information. They include a developer-
22+
* facing message and title, as well as a separate user-facing message. The
23+
* user-facing message is displayed to the end user either as an HTML page or
24+
* a JSON response.
2625
*/
2726
final class UserMessageExceptionHandler extends ExceptionHandler
2827
{
29-
#[Inject]
30-
protected AlertStream $alert;
31-
3228
/**
3329
* Don't log theses exceptions.
3430
*/
@@ -52,36 +48,4 @@ protected function determineStatusCode(ServerRequestInterface $request, Throwabl
5248
{
5349
return intval($exception->getCode());
5450
}
55-
56-
/**
57-
* {@inheritdoc}
58-
*
59-
* Adds the exception message to the alert stream.
60-
*/
61-
public function handle(ServerRequestInterface $request, Throwable $exception): ResponseInterface
62-
{
63-
if ($exception instanceof UserMessageException) {
64-
$title = $this->translateExceptionPart($exception->getTitle());
65-
$description = $this->translateExceptionPart($exception->getDescription());
66-
$this->alert->addMessage('danger', "$title: $description");
67-
}
68-
69-
return parent::handle($request, $exception);
70-
}
71-
72-
/**
73-
* Translate a string or UserMessage to a string.
74-
*
75-
* @param string|UserMessage $message
76-
*
77-
* @return string
78-
*/
79-
protected function translateExceptionPart(string|UserMessage $message): string
80-
{
81-
if ($message instanceof UserMessage) {
82-
return $this->translator->translate($message->message, $message->parameters);
83-
}
84-
85-
return $this->translator->translate($message);
86-
}
8751
}

‎packages/sprinkle-core/app/src/ServicesProvider/ErrorHandlerService.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@
2121
use UserFrosting\Sprinkle\Core\Error\Handler\UserMessageExceptionHandler;
2222
use UserFrosting\Sprinkle\Core\Exceptions\Contracts\UserMessageException;
2323

24+
/**
25+
* Note: Starting with UserFrosting 6.0, `UserMessageException` instances are
26+
* no longer sent to the Alert Stream. To restore the behavior from
27+
* UserFrosting 5, use the `AlertStreamExceptionHandler` instead.
28+
*/
2429
class ErrorHandlerService implements ServicesProviderInterface
2530
{
2631
public function register(): array

‎packages/sprinkle-core/app/tests/Integration/Csrf/CsrfGuardMiddlewareTest.php‎

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
use Psr\Http\Message\ResponseInterface as Response;
1616
use Slim\App;
1717
use Slim\Views\Twig;
18-
use UserFrosting\Alert\AlertStream;
1918
use UserFrosting\Config\Config;
2019
use UserFrosting\Routes\RouteDefinitionInterface;
2120
use UserFrosting\Session\Session;
@@ -42,10 +41,6 @@ public function setUp(): void
4241

4342
public function testFailCsrf(): void
4443
{
45-
/** @var AlertStream */
46-
$ms = $this->ci->get(AlertStream::class);
47-
$ms->resetMessageStream();
48-
4944
// Create request with method and url and fetch response
5045
$request = $this->createJsonRequest('POST', '/csrf');
5146
$response = $this->handleRequest($request);
@@ -54,11 +49,6 @@ public function testFailCsrf(): void
5449
$this->assertResponseStatus(400, $response);
5550
$this->assertJsonResponse('Bad Request', $response, 'title');
5651
$this->assertJsonResponse('Missing CSRF token. Try refreshing the page and then submitting again?', $response, 'description');
57-
58-
// Test message
59-
$messages = $ms->getAndClearMessages();
60-
$this->assertCount(1, $messages);
61-
$this->assertSame('danger', end($messages)['type']); // @phpstan-ignore-line
6252
}
6353

6454
public function testSuccessCsrf(): void
@@ -115,10 +105,6 @@ public function testCsrfBlacklist(): void
115105

116106
public function testCsrfStorage(): void
117107
{
118-
/** @var AlertStream */
119-
$ms = $this->ci->get(AlertStream::class);
120-
$ms->resetMessageStream();
121-
122108
/** @var Config */
123109
$config = $this->ci->get(Config::class);
124110
$csrfKey = $config->getString('session.keys.csrf');
@@ -133,11 +119,6 @@ public function testCsrfStorage(): void
133119

134120
// Assert response status & body
135121
$this->assertResponseStatus(400, $response);
136-
137-
// Test message
138-
$messages = $ms->getAndClearMessages();
139-
$this->assertCount(1, $messages);
140-
$this->assertSame('danger', end($messages)['type']); // @phpstan-ignore-line
141122
}
142123

143124
public function testTwigCsrf(): void

‎packages/sprinkle-core/app/tests/Integration/Error/Handler/AlertStreamExceptionHandlerTest.php‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/*
6+
* UserFrosting Core Sprinkle (http://www.userfrosting.com)
7+
*
8+
* @link https://github.com/userfrosting/sprinkle-core
9+
* @copyright Copyright (c) 2013-2024 Alexander Weissman & Louis Charette
10+
* @license https://github.com/userfrosting/sprinkle-core/blob/master/LICENSE.md (MIT License)
11+
*/
12+
13+
namespace UserFrosting\Sprinkle\Core\Tests\Integration\Error\Handler;
14+
15+
use Slim\App as SlimApp;
16+
use UserFrosting\Alert\AlertStream;
17+
use UserFrosting\Routes\RouteDefinitionInterface;
18+
use UserFrosting\Sprinkle\Core\Core;
19+
use UserFrosting\Sprinkle\Core\Error\ExceptionHandlerMiddleware;
20+
use UserFrosting\Sprinkle\Core\Error\Handler\AlertStreamExceptionHandler;
21+
use UserFrosting\Sprinkle\Core\Exceptions\UserFacingException;
22+
use UserFrosting\Sprinkle\Core\Tests\CoreTestCase as TestCase;
23+
use UserFrosting\Support\Message\UserMessage;
24+
25+
/**
26+
* Test the handler for AlertStreamExceptionHandler.
27+
*/
28+
class AlertStreamExceptionHandlerTest extends TestCase
29+
{
30+
protected string $mainSprinkle = AlertStreamExceptionTestSprinkle::class;
31+
32+
protected function setUp(): void
33+
{
34+
parent::setUp();
35+
36+
// Register the AlertStreamExceptionHandler
37+
$handler = $this->ci->get(ExceptionHandlerMiddleware::class);
38+
$handler->registerHandler(UserFacingException::class, AlertStreamExceptionHandler::class, true);
39+
}
40+
41+
public function testHTML(): void
42+
{
43+
/** @var AlertStream */
44+
$ms = $this->ci->get(AlertStream::class);
45+
$ms->resetMessageStream();
46+
47+
// Create request with method and url and fetch response
48+
$request = $this->createRequest('GET', '/UserFacingException');
49+
$response = $this->handleRequest($request);
50+
51+
// Assert response status & body
52+
$this->assertResponseStatus(400, $response);
53+
54+
// Test message
55+
$messages = $ms->getAndClearMessages();
56+
$this->assertCount(1, $messages);
57+
$this->assertSame('danger', end($messages)['type']);
58+
$this->assertSame('Foo: Bar is bar', end($messages)['message']);
59+
}
60+
61+
public function testJson(): void
62+
{
63+
// Create request with method and url and fetch response
64+
$request = $this->createJsonRequest('GET', '/UserFacingException');
65+
$response = $this->handleRequest($request);
66+
67+
// Assert response status & body
68+
$this->assertResponseStatus(400, $response);
69+
$this->assertJsonResponse([
70+
'title' => 'Foo',
71+
'description' => 'Bar is bar',
72+
'status' => 400
73+
], $response);
74+
75+
// Test message
76+
/** @var AlertStream */
77+
$ms = $this->ci->get(AlertStream::class);
78+
$messages = $ms->getAndClearMessages();
79+
$this->assertCount(1, $messages);
80+
$this->assertSame('danger', end($messages)['type']);
81+
$this->assertSame('Foo: Bar is bar', end($messages)['message']);
82+
}
83+
}
84+
85+
class AlertStreamExceptionTestRoutes implements RouteDefinitionInterface
86+
{
87+
public function register(SlimApp $app): void
88+
{
89+
$app->get('/UserFacingException', function () {
90+
$e = new UserFacingException();
91+
$e->setTitle('Foo');
92+
$e->setDescription(new UserMessage('Bar is {{foo}}', ['foo' => 'bar']));
93+
throw $e;
94+
});
95+
}
96+
}
97+
98+
class AlertStreamExceptionTestSprinkle extends Core
99+
{
100+
public function getRoutes(): array
101+
{
102+
return [
103+
AlertStreamExceptionTestRoutes::class,
104+
];
105+
}
106+
}

0 commit comments

Comments
 (0)