Page MenuHomePhabricator

Thumbor should return informative and nice-looking errors
Closed, ResolvedPublic

Description

Tornado is capable of returning custom html for errors, this should be possible in Thumbor with an upstream change or a monkey patch.

These new errors should be using the unified layout found in T113114: Make all wiki-facing error pages consistent

It might be easier and more desirable to actually have Varnish be in charge of turning the body-free error coming from Thumbor into the informative, better-looking one.

Event Timeline

Change 364271 had a related patch set uploaded (by Gilles; owner: Gilles):
[mediawiki/vagrant@master] Provide fancy error page for Thumbor 429s

https://gerrit.wikimedia.org/r/364271

Change 364271 merged by jenkins-bot:
[mediawiki/vagrant@master] Provide fancy error page for Thumbor 429s

https://gerrit.wikimedia.org/r/364271

For 404 errors, the problem is a bit more severe. Regardless of Swift pages looking very classic (bank serif-font page with H1 with paragraph), since the roll out of Thumbor, the 404 handling of thumbnails has regressed somewhat. It used to be a MediaWiki-generated 404 error (similarly pale, but at least something).

But now the 404 not found handling yields an empty response that instead gets handled by the browser as a native error which rather took me by surprise (feels like a network error).

Y7UQES9.png (1×1 px, 206 KB)

It's not more severe, it's exactly the same problem. Lack of body applies to all error responses.

Change 364953 had a related patch set uploaded (by Gilles; owner: Gilles):
[mediawiki/vagrant@master] Apply custom error to Thumbor only, for all codes it can return

https://gerrit.wikimedia.org/r/364953

Change 364953 merged by jenkins-bot:
[mediawiki/vagrant@master] Apply custom error to Thumbor only, for all codes it can return

https://gerrit.wikimedia.org/r/364953

We could perhaps go after this in the most-general sense. In our common VCL (across all clusters), if we get an error from a backend ([45]xx) which has no body content, we can always turn that into a synth() response and probably add missing reason text in most cases as well (varnish does that by default when you re-set the status code). This way any app that sends errors with empty bodies gets converted to the standardized error templates that Varnish already has.

Change 365587 had a related patch set uploaded (by Gilles; owner: Gilles):
[mediawiki/vagrant@master] Apply fallback Varnish error pages to all bodiless errors

https://gerrit.wikimedia.org/r/365587

Change 365589 had a related patch set uploaded (by Gilles; owner: Gilles):
[operations/puppet@production] Serve a synth error page when error body is empty in Varnish

https://gerrit.wikimedia.org/r/365589

Change 365587 merged by jenkins-bot:
[mediawiki/vagrant@master] Apply fallback Varnish error pages to all bodiless errors

https://gerrit.wikimedia.org/r/365587

Change 365589 merged by Ema:
[operations/puppet@production] Serve a synth error page when error body is empty in Varnish

https://gerrit.wikimedia.org/r/365589