We recently had a case where an embedded image in a node caused a PDF rendering error. This showed up as a fatal crash (effectively a WSOD), since the DomPdf plugin turns rendering errors into exceptions.
An error that can reasonably be caused by content editors should probably be presented in a slightly friendlier way instead of seemingly "crashing the site" as far as end users are concerned. Eg. instead of throwing the exception up to the kernel, the controller at /entityprint/type/id could render an error page explaining that the PDF download is unavailable.
(Furthermore, depending on use case it would be desirable to present the PDF, such as it is, even if there were errors during rendering. If there is an unavailable image, having the PDF with an "image unavailable" box would be much better for diagnosing than refusing the serve it at all. This could be a configurable option, maybe per entity/bundle type.)
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | 0001-Issue-2845221-Catch-rendering-exceptions-in-controll.patch | 1.6 KB | cburschka |
Comments
Comment #2
cburschkaUpdate: The controller already catches PdfEngineException, but only while loading the plugin. The actual rendering happens outside the try-catch block, even though it can trigger the exception as well. I suspect this is a bug.
Comment #3
benjy commentedAre you on the latest version of the 2.x branch? All PrintEngineException exceptions should be caught by PrintEngineExceptionSubscriber.
The Dompdf implementation attempt to catch all errors, must maybe it needs a try/catch around the doRender() call.
Comment #4
cburschkaSorry, I meant to pick 1.3. I guess the entire flow has significantly evolved in 2.x.
For 1.x, a very easy fix would be to move the response into the try block in EntityPrintController::viewPdf().
Comment #5
benjy commentedI'll commit a patch to fix this in the 1.x branch but beware that in the near future i'm going to mark that branch unsupported. Everything and more is now in the 2.x branch and there should be a working upgrade path. If you have any issues updating, please create more issues :)
Comment #6
cburschkaNo worries - once there is a stable 2.x release, we'll simply update. :)
In the meantime, this is the patch I used.
Comment #7
benjy commentedCan you write a test for this, there is an example in the 2.x branch that we could back port.
Comment #8
benjy commentedNW for the tests.
Comment #9
benjy commentedI'm going to commit this patch without tests because I plan to mark the 1.x branch unsupported in the not so distant future. Pretty much as soon as the 2.x branch has a stable release.