Decorate standard error.html.twig template #27071

zorn-v · 2018-04-27T07:33:33Z

Q	A
Branch?	3.4
Bug fix?	no
New feature?	no
BC breaks?	no
Deprecations?	no
Fixed tickets	#22964
License	MIT

zorn-v · 2018-04-28T12:19:17Z

It does not resolve GLOBAL problems that discussed in appropriate tickets. It just brings nice default error page for standard symfony installation in prod env.

zorn-v · 2018-04-30T16:28:34Z

Guys, it just update OLD (check it out) ugly standard twig template. Nothing more.

jvasseur · 2018-04-30T16:38:45Z

I'm 👎 because these are templates that are shown in production environment so we should show something as neutral as possible (they should be overridden on each projects to match your site style).

zorn-v · 2018-04-30T16:41:46Z

You prefer to see "White page with 'Oops'" ?
Some projects no need to define their own error pages. But symfony default is disgrace.

sstok · 2018-04-30T17:00:54Z

At least remove the exception-ghost as this is not really an exception, and non-technical users might be confused about this.

zorn-v · 2018-04-30T17:07:02Z

At least remove the exception-ghost

Yep, I expected it 😄
Ghost gives charm. If it were is no difficult to change "exception" to "error" I'd did it of course.
I can try to remove "exception" cloud at all.

derrabus · 2018-04-30T17:08:19Z

You prefer to see "White page with 'Oops' ?"

Yes, mainly because the framework should not expose itself in prod. That exception ghost and the page layout you chose do exactly that.

Also, that error page is end-user facing. That user probably does not know what an exception is. The fact that Symfony uses exceptions to produce 40x/50x error pages is an implementation detail that the original layout hides. The red color indicates that the user did something wrong, which is not necessarily the case.

But symfony default is disgrace.

Please try to use a more respectful language.

zorn-v · 2018-04-30T17:10:32Z

Yes, mainly because the framework should not expose itself in prod.

And users of freamework says. It is UGLY. Just coz dont configure error pages...

zorn-v · 2018-04-30T17:13:02Z

When I show error page (not configured 404) from silex, I was asked - "Why it is not in symfony ?"

nicolas-grekas · 2018-04-30T17:16:40Z

the framework should not expose itself in prod

technically, the framework already exposes itself: the basic page is still something specific to Symfony, if one were to build heuristics to snif which code is generating this page.

I like the goal here personally.

derrabus · 2018-04-30T17:28:22Z

the basic page is still something specific to Symfony, if one were to build heuristics

Yes, and even without the error pages, you can probably tell by analyzing runtime behavior like HTTP headers etc. My comment was not security-related.

I'd rather like avoid to use Symfony branding on end-user facing templates which the exception ghost and the layout with the read header are. Users seeing that page should get the impression that there's something broken with the application they're using. They should not get the impression of a "broken Symfony", imho.

I like the goal here personnaly.

Agreed. I'm not against changing the default error page in general. I just have concerns regarding the layout proposed in this PR.

zorn-v · 2018-04-30T17:33:38Z

I'd rather like avoid to use Symfony-branding on end-user

Why ? Is it not cool, knowing that some site using symfony ? )
Just because error pages. (as now BTW)

Just changed template. At all

zorn-v · 2018-04-30T17:41:02Z

BTW. I like 'branding' of symfony.
See what you can take, and that it is better to send to long journey 😄

zorn-v · 2018-04-30T18:29:30Z

I'd rather like avoid to use Symfony branding

Nah. Is "Oops" white page not "Symfony branding" ? Disgrace ?

derrabus · 2018-04-30T18:43:44Z

Disgrace ?

Again, I don't think calling the work of someone a "disgrace" is a helpful contribution to a discussion.

nicolas-grekas · 2018-04-30T18:45:42Z

Shouldn't we just remove the ghost and keep the rest? It'd look OK to me.

javiereguiluz · 2018-04-30T18:51:29Z

I disagree with this proposal too. The reason is that I don't want to make users think that it's OK to not customize the error pages in their apps. That's why our default error pages in prod should look like real and finished error pages. Also, not leaking any detail or information about Symfony is nice ... although some people can now associate the "Oops ..." message to Symfony.

nicolas-grekas · 2018-04-30T18:53:10Z

Either we refuse this proposal, or we fix #22964. Right now we have a mix.

zorn-v · 2018-04-30T18:54:09Z

I remove "exception" cloud, when I be able.

don't want to make users think that it's OK to not customize the error pages in their apps

And because of this we provide ugly pages (BTW, in SILEX it MUCH BETTER).

zorn-v · 2018-05-03T01:48:45Z

nicolas-grekas · 2018-05-04T00:18:37Z

@zorn-v looks great to me. Could you please update the ghost in the Debug component also? The word exception is used in prod there also, but this is not desired.

zorn-v · 2018-05-04T01:26:57Z

Is it ok, that typehints are removed from constructor for 3.4 compat ?

nicolas-grekas · 2018-05-04T01:29:19Z

Yes it is. Is it required? Tests fail without?

zorn-v · 2018-05-04T01:40:15Z

Nope, but I think I make mistake when used master branch as base instead of 3.4

nicolas-grekas · 2018-05-04T02:10:00Z

This is not a bug fix so master is correct :)

zorn-v · 2018-05-04T17:54:38Z

Yep, this is not bugfix. But WHY users of "old" versions of symfony (coz not php 7.1 on server) should see white page ?

nicolas-grekas · 2018-05-04T18:04:19Z

Because we cannot support everyone. We have policies and they help us keep the maintenance burden handleable.

zorn-v · 2018-05-04T18:07:57Z

I agree. One time step against... and we have anarchy.
Just forgot.

nicolas-grekas

With minor comment.
We must display something, and however raw we may want it, it doesn't have to be ugly.
Also the current situation is inconsistent, with the themed page being displayed in prod also by the Debug component, while the B&W one is when Twig is enabled.

nicolas-grekas · 2018-05-04T18:12:13Z

src/Symfony/Component/Debug/ExceptionHandler.php

@@ -36,7 +36,7 @@ class ExceptionHandler
    private $caughtLength;
    private $fileLinkFormat;

-    public function __construct(bool $debug = true, string $charset = null, $fileLinkFormat = null)
+    public function __construct($debug = true, $charset = null, $fileLinkFormat = null)


Should be reverted

Eeeh. I asked about this )

#27071 (comment)

No problem, BTW

That's the answer :)
The composer.json of the component states that php7.1 is required, so scalar type hints can be used.

zorn-v · 2018-05-04T18:24:57Z

We must display something, and however raw we may want it, it doesn't have to be ugly.

EXACTLY.
#27071 (comment)
I didn't pay much attention before I was asked. WHY I MUST to configure error page if I dont want ?

javiereguiluz · 2018-05-22T07:16:47Z

I'm still 👎 about this proposal. The proposed decoration feels "out of place" for prod pages. The "Symfony ghost" and the rest of elements have been created for the dev pages.

If we want to display a non-ugly error page, we could do something like this, which is totally agnostic and doesn't leak any Symfony design or information:

javiereguiluz · 2018-05-22T09:36:07Z

In case you think my last proposal is "too simple" or "not ready for prod", this is Google's 404 error page in production:

zorn-v · 2018-05-22T15:10:22Z

The "Symfony ghost" and the rest of elements have been created for the dev pages.

Smells like discrimination. Users of my projects not worthy cool error page if I lazy to override it ?
Is not "like a google" MUST override standard error page if they will be doing project with symfony ?

andrewmy · 2018-05-25T14:50:31Z

Users of my projects not worthy cool error page if I lazy to override it ?

Yep.

joshlopes · 2018-05-25T16:36:37Z

👎 Mostly because:

Red is generally a bad colour to give to people that don't understand, they would probably think either the website is a scam / fake / wrong, or it was actually the site intention to give you that page.
For us DEVs we can actually associate that template to a dev / test enviromnent if i saw that page in production i would think i had debug enabled or some other env was being used, which would be completely misleading.
I prefer white (happy pages) even to show an error - which needs to be imo "naked" and clear to the user it was not part of my website.

I'm happy with @javiereguiluz proposal, I think google got it right, and nothing better to show to the users than something they are used to.

zorn-v · 2018-05-28T15:42:41Z

👎 Mostly because:

You could override it, no ?
Just like now.

zorn-v · 2018-05-28T16:53:13Z

I'm happy with @javiereguiluz proposal,

But nobody seems this problem before, yaah.

joshlopes · 2018-05-29T12:17:57Z

@zorn-v please don't misunderstand me/us. We are grateful for your contribution.

AFAIK PR's are reviewed by the community, and i think something that doesn't please the majority, usually the author has to re-consider and take in consideration others opinions. I think the majority, currently, is engaged with the idea (so good job!) but the template used is inadequate.

You could override it, no?

This also applies to you, everyone can override... But this PR will change everyone's template (no one would merge something, only to have it overridden on millions of projects...). Not that i would override this template, but for the reasons i stated i won't be happy with this template either.

But nobody seems this problem before, yaah.

To be honest, for the reasons above... the blank pages suffice for my needs. I think i can speak for all of us, when i say, we try to avoid errors (so styling this error page has never been a priority for many of us). The current blank page, states that there is an error in a way i am sure the user will understand something was off with his request. This PR could fix some issues like sending the user back to homepage.

It's up for the authors of symfony to decide, as they have the last word. But currently, it would be a lot easier just to change it to match the majority expectations.

If i may ask you, what are the reasons for you to keep this red template? (similar to debug / dev pages) ?

zorn-v · 2018-05-29T13:54:04Z

To be honest, for the reasons above... the blank pages suffice for my needs.

Yep. Same for me. But I was asked of my friend wich I learn for symfony. Why is my old project (in silex) have cool error page, but symfony one not.

joshlopes · 2018-05-29T14:35:24Z

@zorn-v I'm sorry but that's not a good reason to keep a red template (re-use the debug / dev template), so we can have a "cool" error page. I think in such special cases you would indeed override the template in your project...

But since you already had all this work, what about doing what @javiereguiluz purposed above? or something similar? that would be great, and if you ask me you would be cool as any sf contributor 😎 , here take yours 🕶

javiereguiluz · 2018-06-22T15:47:24Z

@zorn-v as you can see in the previous comments, most people are in favor of a simpler error page design. This doesn't mean that you are wrong; it's just that we think differently about this.

For this reason I've made some changes in your original pull request to implement this simpler design. This is how it looks:

It uses the same font stack as used by modern CSS frameworks and it uses the same red color shade as the "danger" text color of Bootstrap 4, ensuring a right contrast ratio. Thanks!

javiereguiluz · 2018-06-25T07:15:46Z

@zorn-v as you requested, I'm closing this pull request and creating a new one with the latest changes: #27699. Thanks!

…reguiluz) This PR was merged into the 4.2-dev branch. Discussion ---------- Redesigned the default error page in production | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #22964 | License | MIT | Doc PR | - This continues the work done in #27071. Before/After comparison: ![before-after-prod-error-pages](https://user-images.githubusercontent.com/73419/41844084-6e132208-786f-11e8-97c9-53602395e231.png) Commits ------- 1df8b3e Redesigned the default error page in production

zorn-v · 2020-12-06T12:19:41Z

Thanks, guys 😄

Decorate standard error.html.twig template

5d71ca8

carsonbot added the Status: Needs Review label Apr 27, 2018

nicolas-grekas added this to the next milestone Apr 27, 2018

Changed ghost image

0a32708

Ghost for prod env in debug component

e9b2643

zorn-v force-pushed the patch-1 branch from 393d4bf to e9b2643 Compare May 4, 2018 01:26

nicolas-grekas approved these changes May 4, 2018

View reviewed changes

carsonbot added Status: Reviewed and removed Status: Needs Review labels May 4, 2018

Typehints fix

9c5ad07

javiereguiluz mentioned this pull request May 22, 2018

[Debug] is the new design desired when the Debug component is used in non-debug mode, and standalone? #22964

Closed

javiereguiluz added 2 commits June 22, 2018 17:43

Simplified the proposed design

c5cd98e

Reverted some unneeded changes

7146579

javiereguiluz mentioned this pull request Jun 25, 2018

Redesigned the default error page in production #27699

Merged

javiereguiluz closed this Jun 25, 2018

nicolas-grekas modified the milestones: next, 4.2 Nov 1, 2018

Decorate standard error.html.twig template #27071

Decorate standard error.html.twig template #27071

Conversation

zorn-v commented Apr 27, 2018 • edited by nicolas-grekas Loading

zorn-v commented Apr 28, 2018

zorn-v commented Apr 30, 2018

jvasseur commented Apr 30, 2018

zorn-v commented Apr 30, 2018 • edited Loading

sstok commented Apr 30, 2018

zorn-v commented Apr 30, 2018 • edited Loading

derrabus commented Apr 30, 2018

zorn-v commented Apr 30, 2018

zorn-v commented Apr 30, 2018

nicolas-grekas commented Apr 30, 2018 • edited Loading

derrabus commented Apr 30, 2018 • edited Loading

zorn-v commented Apr 30, 2018 • edited Loading

zorn-v commented Apr 30, 2018

zorn-v commented Apr 30, 2018 • edited Loading

derrabus commented Apr 30, 2018

nicolas-grekas commented Apr 30, 2018

javiereguiluz commented Apr 30, 2018

nicolas-grekas commented Apr 30, 2018

zorn-v commented Apr 30, 2018

zorn-v commented May 3, 2018

nicolas-grekas commented May 4, 2018

zorn-v commented May 4, 2018

nicolas-grekas commented May 4, 2018

zorn-v commented May 4, 2018

nicolas-grekas commented May 4, 2018

zorn-v commented May 4, 2018

nicolas-grekas commented May 4, 2018

zorn-v commented May 4, 2018

nicolas-grekas left a comment • edited Loading

Choose a reason for hiding this comment

nicolas-grekas May 4, 2018

Choose a reason for hiding this comment

zorn-v May 4, 2018

Choose a reason for hiding this comment

zorn-v May 4, 2018

Choose a reason for hiding this comment

zorn-v May 4, 2018

Choose a reason for hiding this comment

nicolas-grekas May 4, 2018

Choose a reason for hiding this comment

zorn-v commented May 4, 2018

javiereguiluz commented May 22, 2018

javiereguiluz commented May 22, 2018

zorn-v commented May 22, 2018 • edited Loading

andrewmy commented May 25, 2018

joshlopes commented May 25, 2018 • edited Loading

zorn-v commented May 28, 2018 • edited Loading

zorn-v commented May 28, 2018

joshlopes commented May 29, 2018 • edited Loading

zorn-v commented May 29, 2018

joshlopes commented May 29, 2018

javiereguiluz commented Jun 22, 2018

javiereguiluz commented Jun 25, 2018

zorn-v commented Dec 6, 2020

zorn-v commented Apr 27, 2018 •

edited by nicolas-grekas

Loading

zorn-v commented Apr 30, 2018 •

edited

Loading

zorn-v commented Apr 30, 2018 •

edited

Loading

nicolas-grekas commented Apr 30, 2018 •

edited

Loading

derrabus commented Apr 30, 2018 •

edited

Loading

zorn-v commented Apr 30, 2018 •

edited

Loading

zorn-v commented Apr 30, 2018 •

edited

Loading

nicolas-grekas left a comment •

edited

Loading

zorn-v commented May 22, 2018 •

edited

Loading

joshlopes commented May 25, 2018 •

edited

Loading

zorn-v commented May 28, 2018 •

edited

Loading

joshlopes commented May 29, 2018 •

edited

Loading