improve error message

[dbu]

when i manage to pass an empty path into the imagine_filter twig function (e.g. i have a phpcr Image document that is null). now instead of getting some message like "Invalid image source" i get "An exception has been thrown during the rendering of a template ("Parameter "path" for route "_imagine_home_slideshow" must match ".+" ("" given) to generate a corresponding URL.")"

the last step is in CacheManager::generateUrl - is it sane to assume there that if the path is empty we do have a problem? or are there use cases where path can legally be empty?

i am happy to do a PR for this, but wanted to ask first if it makes sense at all.

[havvg]

I don't see any use case for an empty path on this route.

[Tobion]

You won't even get this exception in production environment using SE config because the requirement check does not take place then. It instead will generate such a route with empty path anyway.

[dbu]

hm, but then is it better if we silently ignore the issue or throw an
exception? i guess in dev its nice to see there is an issue. but in
production i prefer a page to not show an image rather than have a
server error and not show up at all.
should we check if kernel.debug is active and only then validate the path?

[havvg]

@Tobion Are you sure on this? It sounds scary and wrong to me, I'm not using SE config, which option is it that enables this behavior?

[havvg]

Well, found it myself; strict_requirements it is.

[havvg]

I would suggest to result in a 404 if the path is empty. A better solution would be to add an event to resolve this 404 if the user likes to, e.g. fallback image on filter_set basis.

[dbu]

so we actually would change the route path requirements to .* instead of
.+, right? and then handle the fact that we are called with no path and
result in a 404?

[Tobion]

Well this check is only disabled for generating urls for performance reasons, not for matching. So a wrongly generated url would result in 404 anyway (unless another route matches).

[havvg]

Ah, that makes more sense. Then I would stick with the first solution, to make the exception more readable (in case path is empty), so no invalid URL is generated.

[makasim]

moving to 1.1 milestone as 0.20 is not supported any more.

[michellesanver]

Does this still make sense for you @dbu ?

[dbu]

lets drop it :-)