Skip to content
This repository has been archived by the owner on Mar 25, 2024. It is now read-only.

Throw NotFoundHttpException when debug is disabled #13

Merged
merged 5 commits into from
Apr 12, 2017

Conversation

salkhwlani
Copy link
Contributor

@salkhwlani salkhwlani commented Apr 10, 2017

Throw NotFoundHttpException when debug is disabled and pass string in
URL

for example http://site.com/user/1844379023 work fine but
http://site.com/user/test throw InvalidArgumentException

Throw NotFoundHttpException when debug is disabled and pass string in
URL

for example `http://site.com/user/1844379023` work find but
`http://site.com/user/test` throw InvalidArgumentException
Copy link
Owner

@Propaganistas Propaganistas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Good addition. Please process my feedback. Could you also add tests?

try {
$value = $this->container->make('fakeid')->decode($value);
} catch (\InvalidArgumentException $e) {
throw config('app.debug') ? new \InvalidArgumentException($e->getMessage()) : new NotFoundHttpException;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to reconstruct an InvalidArgumentException. Just rethrow $e:

throw config('app.debug') ? $e : new NotFoundHttpException;

Copy link
Contributor Author

@salkhwlani salkhwlani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you right, check now

@Propaganistas
Copy link
Owner

The tests are failing. Please fix and also revert the tests file structure & Route facade use.

@Propaganistas Propaganistas merged commit 13cecfd into Propaganistas:master Apr 12, 2017
@Propaganistas
Copy link
Owner

Thanks. A new release has been tagged: 2.0.6

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants