Handle post rendering errors to avoid bricking (#3061)

Whether it's due to corrupted content, missing tags, caching issues, or other assorted reasons, post content can't be rendered. Currently, this results in an exception that crashes the entire forum and is hard to debug. Instead, we should log the error and show an indicator message that rendering has failed.

Co-authored-by: Sami Mazouz <sychocouldy@gmail.com>
Co-authored-by: David Wheatley <hi@davwheat.dev>
This commit is contained in:
Alexander Skvortsov 2021-10-14 14:30:18 -04:00 committed by GitHub
parent 13b312d445
commit 35986a050c
6 changed files with 116 additions and 1 deletions

View File

@ -10,9 +10,11 @@ Object.assign(Post.prototype, {
createdAt: Model.attribute('createdAt', Model.transformDate),
user: Model.hasOne('user'),
contentType: Model.attribute('contentType'),
content: Model.attribute('content'),
contentHtml: Model.attribute('contentHtml'),
renderFailed: Model.attribute('renderFailed'),
contentPlain: computed('contentHtml', getPlainContent),
editedAt: Model.attribute('editedAt', Model.transformDate),

View File

@ -100,6 +100,7 @@ export default class CommentPost extends Post {
' ' +
classList({
CommentPost: true,
'Post--renderFailed': post.renderFailed(),
'Post--hidden': post.isHidden(),
'Post--edited': post.isEdited(),
revealContent: this.revealContent,

View File

@ -185,6 +185,10 @@
}
}
.Post--renderFailed {
background-color: @control-danger-bg;
}
.Post--hidden {
.Post-header, .Post-header a, .PostUser h3, .PostUser h3 a {
color: @muted-more-color;

View File

@ -527,6 +527,7 @@ core:
payload_too_large_message: The request payload was too large.
permission_denied_message: You do not have permission to do that.
rate_limit_exceeded_message: You're going a little too quickly. Please try again in a few seconds.
render_failed_message: Sorry, we encountered an error while displaying this content. If you're a user, please try again later. If you're an administrator, take a look in your Flarum log files for more information.
# These translations are used in the loading indicator component.
loading_indicator:

View File

@ -9,12 +9,30 @@
namespace Flarum\Api\Serializer;
use Exception;
use Flarum\Foundation\ErrorHandling\LogReporter;
use Flarum\Post\CommentPost;
use Flarum\Post\Post;
use InvalidArgumentException;
use Symfony\Contracts\Translation\TranslatorInterface;
class BasicPostSerializer extends AbstractSerializer
{
/**
* @var LogReporter
*/
protected $log;
/**
* @var TranslatorInterface
*/
protected $translator;
public function __construct(LogReporter $log, TranslatorInterface $translator)
{
$this->log = $log;
$this->translator = $translator;
}
/**
* {@inheritdoc}
*/
@ -41,7 +59,14 @@ class BasicPostSerializer extends AbstractSerializer
];
if ($post instanceof CommentPost) {
$attributes['contentHtml'] = $post->formatContent($this->request);
try {
$attributes['contentHtml'] = $post->formatContent($this->request);
$attributes['renderFailed'] = false;
} catch (Exception $e) {
$attributes['contentHtml'] = $this->translator->trans('core.lib.error.render_failed_message');
$this->log->report($e);
$attributes['renderFailed'] = true;
}
} else {
$attributes['content'] = $post->content;
}

View File

@ -0,0 +1,82 @@
<?php
/*
* This file is part of Flarum.
*
* For detailed copyright and license information, please view the
* LICENSE file that was distributed with this source code.
*/
namespace Flarum\Tests\integration\api\posts;
use Carbon\Carbon;
use Flarum\Testing\integration\RetrievesAuthorizedUsers;
use Flarum\Testing\integration\TestCase;
class ShowTest extends TestCase
{
use RetrievesAuthorizedUsers;
/**
* @inheritDoc
*/
protected function setUp(): void
{
parent::setUp();
$this->prepareDatabase([
'discussions' => [
['id' => 1, 'title' => 'Discussion with post', 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'first_post_id' => 1, 'comment_count' => 1, 'is_private' => 0],
],
'posts' => [
['id' => 1, 'discussion_id' => 1, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'type' => 'comment', 'content' => '<t><p>valid</p></t>'],
['id' => 2, 'discussion_id' => 1, 'created_at' => Carbon::now()->toDateTimeString(), 'user_id' => 2, 'type' => 'comment', 'content' => '<tMALFORMED'],
],
'users' => [
$this->normalUser(),
]
]);
}
/**
* @test
*/
public function properly_formatted_post_rendered_correctly()
{
$response = $this->send(
$this->request('GET', '/api/posts/1', [
'authenticatedAs' => 2,
])
);
$this->assertEquals(200, $response->getStatusCode());
$body = (string) $response->getBody();
$this->assertJson($body);
$data = json_decode($body, true);
$this->assertEquals($data['data']['attributes']['contentHtml'], '<p>valid</p>');
}
/**
* @test
*/
public function malformed_post_caught_by_renderer()
{
$response = $this->send(
$this->request('GET', '/api/posts/2', [
'authenticatedAs' => 2,
])
);
$this->assertEquals(200, $response->getStatusCode());
$body = (string) $response->getBody();
$this->assertJson($body);
$data = json_decode($body, true);
$this->assertEquals("Sorry, we encountered an error while displaying this content. If you're a user, please try again later. If you're an administrator, take a look in your Flarum log files for more information.", $data['data']['attributes']['contentHtml']);
}
}