implemented getRoutesByNames(), removed bogus try/catch #187
Conversation
lsmith77
commented
Oct 7, 2013
lsmith77
commented
| Q | A |
|---|---|
| Bug fix? | no |
| New feature? | yes |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #142 |
| License | MIT |
| Doc PR | n/a |
| // https://github.com/symfony-cmf/RoutingBundle/issues/143 | ||
| // for example, getting /my//test (note the double /) is just an invalid path | ||
| // and means another router might handle this. | ||
| // but if the PHPCR backend is down for example, we want to alert the user |
There was a problem hiding this comment.
i don't think we can simply remove this. it will lead to an error 500 if i make my url look like /my//test (see the comment) or anything else not valid in phpcr, like /a:b:c. while i agree that its edge cases, it should still not break as the symfony router or whatever other router comes later in the chain could handle those. and instead of a 500 a weird url should lead to a 404.
the only thing with this exception is that its eating away things we care about while developping, like the repository being down.
There was a problem hiding this comment.
err .. this was an accident
There was a problem hiding this comment.
i wanted to remove this from the ORM .. done now
| // also check if there are valid reasons for the orm manager to | ||
| // throw an exception or if we should just not catch it to not hide | ||
| // a severe problem. | ||
| $collection->add($key, $route); |
There was a problem hiding this comment.
this would fix #142 - do we lose anything here or can we never get exceptions except when the db is down?
There was a problem hiding this comment.
no .. Sylius/Sylius#255 (diff)
There was a problem hiding this comment.
great. i added a note to the issue that we fix #142 with this
implemented getRoutesByNames(), removed bogus try/catch
