Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added ability to throw a method not found instead of 404 #591

Merged
merged 1 commit into from
May 19, 2024

Conversation

n0nag0n
Copy link
Collaborator

@n0nag0n n0nag0n commented May 17, 2024

If you are routing a URL and it finds the URL, but not the method, this allows a 405 Method Not Found error to be thrown instead of the expected 404 Not Found.

// Process things normally for before, and then in reverse order for after.
$middlewares = $eventName === Dispatcher::FILTER_BEFORE
? $route->middleware
: array_reverse($route->middleware);
$params = $route->params;
Copy link
Collaborator Author

@n0nag0n n0nag0n May 17, 2024

Choose a reason for hiding this comment

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

This will look better if you remove the whitespace setting in the CR

// Get the previous route and check if the method failed, but the URL was good.
$lastRouteExecuted = $router->executedRoute;
if ($lastRouteExecuted !== null && $lastRouteExecuted->matchUrl($request->url) === true && $lastRouteExecuted->matchMethod($request->method) === false) {
$this->halt(405, 'Method Not Allowed', empty(getenv('PHPUNIT_TEST')));

Choose a reason for hiding this comment

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

Sweet, I like this, so helpful for debugging!

Copy link

@luka97zr luka97zr left a comment

Choose a reason for hiding this comment

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

Thanks for making these changes that fast, so cool!

@n0nag0n n0nag0n merged commit 10165eb into master May 19, 2024
@n0nag0n n0nag0n deleted the method-not-found branch May 19, 2024 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants