silexphp / Silex

[DEPRECATED -- Use Symfony instead] The PHP micro-framework based on the Symfony Components
https://silex.symfony.com
MIT License
3.58k stars 718 forks source link

Missing "_locale" attribute in 404 error page, when no route was matched #1129

Closed ZeeCoder closed 9 years ago

ZeeCoder commented 9 years ago

I couldn't find anything on the issue, but basically I set up my error pages which are multilingual. Now when I throw a NotFound exception manually, everything works fine, but when a route is not matched, like: "/asd", or even "/en/asd", that throws an error: An exception has been thrown during the rendering of a template ("Some mandatory parameters are missing ("_locale") to generate a URL for route(...)

Why is there such a difference, and how can I handle it?

Thanks in advance, and also here is some relevant code I'm using:

$app->error(function (\Exception $e, $code) use ($app) {
    if ($app['debug']) {
        return;
    }

    // Catch all errors and show something useful.
    $templates = array(
        'errors/'.$code.'.html.twig',
        'errors/'.substr($code, 0, 2).'x.html.twig',
        'errors/'.substr($code, 0, 1).'xx.html.twig',
        'errors/default.html.twig',
    );

    return new Response($app['twig']->resolveTemplate($templates)->render(array(
        'code' => $code,
    )), $code);
});
ghost commented 9 years ago

You can try to add the following line to your error handler:

$app['request_context']->setParameters(array('_locale' => $app['locale']));
ZeeCoder commented 9 years ago

Okay, I get a different error now: The merge filter only works with arrays or hashes; NULL and array given in "errors/404.html.twig" at line 3.

I also have this in the error stack: at twig_array_merge(null, array('_locale' => 'en')).

ghost commented 9 years ago

Could you show your templates?

ZeeCoder commented 9 years ago

The only place I'm using the merge filter I think is the menu bar. I don't see why it behaves differently when I want to render the error pages.

{% trans_default_domain 'menu' %}

{% spaceless %}
    <div class="b-menu {{ isWhite is defined and isWhite ? 'b-menu--is-white': '' }} {{ isSticky is defined ? 'b-menu--is-sticky jsm-sticky-menu jsm-sticky-menu__wrapper': '' }} {{ classes is defined ? classes : '' }}">
        <div class="row collapse">
            <div class="column small-12">
                <a href="{{ isSticky is defined ? 'javascript:void(0)' : path('home') }}" class="b-menu__logo {{ isSticky is defined ? 'jsm-scroll-to' : '' }}"
                {% if isSticky is defined %}data-jsm-scroll-to='{"top": 0}'{% endif %}
                >
                    <img
                        src="{{ asset(
                            [
                                'images/logo',
                                ((isWhite is defined and isWhite) ? '_white': ''),
                                '.png'
                            ]|join
                        ) }}"
                        alt="logo" class=""
                        {% if isSticky is defined %}
                            width="49" height="42"
                        {% else %}
                            width="59" height="51"
                        {% endif %}
                    >
                </a>

                <h3 class="b-menu__header">{{ 'header'|trans }}</h3>

                <div class="b-menu__actions-wrapper">
                    <div class="b-menu__lang-wrapper">
                        {% set localeUrlPart = request.locale == 'en' ? 'hu': 'en' %}
                        {% if mobileDetect.isMobile %}
                            {% set localeToChangeFrom = request.locale == 'en' ? 'hu': 'en' %}
                            {% set localeToChangeTo = localeToChangeFrom %}
                        {% else %}
                            {% set localeToChangeFrom = request.locale %}
                            {% set localeToChangeTo = request.locale == 'en' ? 'hu': 'en' %}
                        {% endif %}

                        <a href="{{ path(
                            request.attributes.get('_route'),
                            app.request.attributes.get('_route_params') | merge({
                                _locale: localeUrlPart
                            })
                        ) }}" class="b-menu__lang-btn">{{ localeToChangeFrom }}</a>
                        <a href="{{ path(
                            request.attributes.get('_route'),
                            app.request.attributes.get('_route_params') | merge({
                                _locale: localeUrlPart
                            })
                        ) }}" class="b-menu__lang-btn">{{ localeToChangeTo }}</a>
                    </div>
                    <div class="b-menu__sep"></div>
                    {% include 'partials/hamburger_btn.html.twig' with {
                        classes: (isWhite is defined and isWhite ? 'b-hamburger--white': '')
                    } %}
                </div>
            </div>
        </div>
    </div>
{% endspaceless %}
ghost commented 9 years ago

I could be wrong, but IMO, you should check that type of _route_params is array.

ZeeCoder commented 9 years ago

Okay, I refactored it so that it detects whether the _route_params attribute is iterable or not, and sets it as an empty array if not. (I very much dislike it though, since I don't quite get why I have to hack around this.)

Problems are still present though: app.request.locale is not set, and the error is always presented in the default (en) language.

Oh and btw: Thank you so much for the effort you're making here, I appreciate it greatly. ^^

ghost commented 9 years ago

I very much dislike it though, since I don't quite get why I have to hack around this.

GET /hello: $request->attributes->get('_route_params', null) => null GET /hello/user: $request->attributes->get('_route_params', null) => ['name' => 'user']

app.request.locale is not set

As workaround you can set it manually.

I will try to find right solution later.

ghost commented 9 years ago

I found a cause of the problem. You can get necessary attributes from request only when a requested url corresponds to a route. So you need to set attributes manually, for example.

$app->error(function (\Exception $e, $code) use ($app) {
//    if ($app['debug']) {
//        return;
//    }
    $routeName = 'homepage';
    $route = $app['routes']->get($routeName);
    $app['request']->attributes->set('_locale', $app['locale']);
    $app['request']->attributes->set('_route_params', $route->compile()->getVariables());
    $app['request']->attributes->set('_route', $routeName);

    $templates = array(
        'errors/'.$code.'.html',
        'errors/'.substr($code, 0, 2).'x.html.twig',
        'errors/'.substr($code, 0, 1).'xx.html.twig',
        'errors/default.html.twig',
    );

    return new Response($app['twig']->resolveTemplate($templates)->render(array('code' => $code)), $code);
});
ghost commented 9 years ago

Improved example:

$app->error(function (\Exception $e, $code) use ($app) {
//    if ($app['debug']) {
//        return;
//    }
    if ($code == 404) {
        $routeName = 'homepage';
        $route = $app['routes']->get($routeName);
        $app['request']->attributes->set('_locale', $app['locale']);
        $params = [];
        $defaults = ['_locale' => $app['locale']];
        foreach ($route->compile()->getVariables() as $variable) {
            $params[$variable] = $defaults[$variable];
        }
        $app['request']->attributes->set('_route_params', $params);
        $app['request']->attributes->set('_route', $routeName);
    }

    // 404.html, or 40x.html, or 4xx.html, or error.html
    $templates = array(
        'errors/'.$code.'.html',
        'errors/'.substr($code, 0, 2).'x.html.twig',
        'errors/'.substr($code, 0, 1).'xx.html.twig',
        'errors/default.html.twig',
    );

    return new Response($app['twig']->resolveTemplate($templates)->render(array('code' => $code)), $code);
});
indeyets commented 9 years ago

You can get necessary attributes from request only when a requested url corresponds to a route. So you need to set attributes manually

well, looks like a bug to me

ZeeCoder commented 9 years ago

It also doesn't work for me, I get the same ...missing ("_locale")... error, it's as if it doesn't matter what I do with the $app['request'] Request object.

Even if it worked like this, it would still bug me. I think it's quite common, that a multilingual site needs multilingual error pages. I can't get my head around how is it possible, that this is so hard to achieve? Even if it's this tricky, then it should be at least reflected in the docs, in an "error pages" section or somewhere.

I'm quite a big advocate of Symfony Components in general, so I think there's a good reason for why this is happening, but the issue is that I just can't find anything on the matter, and I can't work out any solution for it.

Hmm, actually I think I know why this is happening: since no route was matched, the Request won't contain any route-specific data. That would be the reason why one has to "hack-in" the normally expected attributes. (Except it doesn't seem to work for me.)

I think this is one of the reasons people sometimes choose Laravel, Yii and whatnot over Silex and Symfony. Simply put the developer-experience can be painful in cases. (Hence the Symfony DX-movement.)

ghost commented 9 years ago

Hmm, actually I think I know why this is happening: since no route was matched, the Request won't contain any route-specific data.

Exactly.

ZeeCoder commented 9 years ago

Exactly.

But I hope I did get my point across? Although I tracked down the source of this issue, I still cannot be satisfied, since nothing really worked so far. Not to mention how much of a nightmare this could be for a developer just trying out the framework. Meeting an issue like this would be an instant deal-breaker when choosing a framework.

Anyway, I'll search for this exact issue about the routing, I think I'll get better results. I'll be back with new info asap, and until then: thanks for the comments so far. :+1:

ZeeCoder commented 9 years ago

Okay, so I've created a temporary solution. It still has serious issues, like: the Request->getLocale() call will always return the default locale, so I still need a proper solution.

Added some comments to the code:

$app->error(function (\Exception $e, $code) use ($app) {
    // See the docs: http://silex.sensiolabs.org/doc/usage.html
    if ($app['debug']) {
        return;
    }

    // Setting the locale in a hacky way.
    // The translator and url_generator will work, and rendering won't complain
    // about the missing "_locale" attribute.
    // But the `Request->getLocale()` call will always return the default ('en')
    // language...

    // Had to get the locale a trickier way, since $app['locale'] is always
    // "en"...
    $locale = explode('/', trim(str_replace(
        $app['request_context']->getBaseUrl(),
        '',
        $_SERVER['REQUEST_URI']
    ), '/'));
    $locale = array_shift($locale);

    // Missing "_locale" attribute solution.
    $app['request_context']->setParameters(array('_locale' => $locale));
    // Translator solution.
    $app['translator']->setLocale($locale);

    // These didn't work at all
    // $app['request']->attributes->set('_locale', $locale);
    // $app['request']->getSession()->set('_locale', $locale);
    // $app['request']->setLocale($app['request']->getSession()->get('_locale', $app['locales'][0]));
    // $app['request_context']->setLocale($locale);

    // Catch all errors and show something useful.
    $templates = array(
        'errors/'.$code.'.html.twig',
        'errors/'.substr($code, 0, 2).'x.html.twig',
        'errors/'.substr($code, 0, 1).'xx.html.twig',
        'errors/default.html.twig',
    );

    return new Response($app['twig']->resolveTemplate($templates)->render(array(
        'code' => $code,
    )), $code);
});
ZeeCoder commented 9 years ago

Came back with an update: it seems, that updating to the still non-stable v2 release should solve my problems. I almost opened up a new issue, because ->before handlers wouldn't execute before error handling, and that caused - yet again - a tremendous amount of pain.

With a new project however I updated to v2-dev, and the before handlers here executed in the proper order.

Now I hope that locale handling will be fixed too, so I'll upgrade the previous project. If it gets fixed just by that, I'll close this thread with my finishing thoughts.

ZeeCoder commented 9 years ago

Also: declaring the default translation ({% trans_default_domain 'domain' %}) locale in an error page didn't work.

A lot got fixed in the new version.

fabpot commented 9 years ago

Closing this one as the locale is determined based on the routing information, which of course are not available when Silex/Symfony cannot find a matching route. Remember that the locale by default is only determined based on the URL path, nothing else. So, if you want to localize your error page, you should "store" the current user locale (I suppose he was already browsing your website somehow) in a cookie.

But, and I think it makes sense, when someone try to access (first time) a page that does not exist, how would you determine which locale to use? Probably via the browser language or something like this, but that's something you need to do yourself (with the help of Request::getPreferredLanguage() for instance).