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

[8.x] Give a more meaningul message when route parameters are missing #35706

Conversation

simensen
Copy link
Contributor

The message for UrlGenerationException when parameters are missing is a nice start, but knowing which parameters are missing can be a huge help in figuring out where something is broken. Especially if a route has multiple possible parameters. Which parameter is actually missing? Are more than one missing? Now we'll know!

For example, say a developer is presented with the following exception message:

Missing required parameters [Route: admin.client.form.show] [URI: admin/client/{client}/form/{webform}]

It would lead them to their controller which might look something like this:

$connectionRequest->serviceUrl = route('admin.client.form.show', [
    'client' => $connectionRequest->client,
    'webform' => $serviceIntake,
]);

... it is not immediately obvious where to start. Is it that the $connectionRequest doesn't have a client? Or is the $serviceIntake set to null for some reason? Or are they both having issues?

With this PR, the exception message would change to one of the following:

  • Missing required parameters [Route: admin.client.form.show] [URI: admin/client/{client}/form/{webform}] [Missing Parameters: client, webform].

  • Missing required parameter [Route: admin.client.form.show] [URI: admin/client/{client}/form/{webform}] [Missing Parameter: client].

  • Missing required parameter [Route: admin.client.form.show] [URI: admin/client/{client}/form/{webform}] [Missing Parameter: webform].

This feedback will save the developer a lot of time trying to figure out which parameters are missing so they can get right to fixing the issue instead of digging more to figure out which parameter is actually causing the problem.


Originally targeted 8.x with this PR, but realized it can go all the way back to 6.x, too. Let me know if you'd like me to retarget another branch or if there is anything I can do to help ensure this makes it into the most versions possible if it is accepted. :)

@taylorotwell
Copy link
Member

Hey Beau, would you mind sending this to 8.x?

@simensen simensen changed the base branch from 6.x to 8.x December 23, 2020 18:21
The message for `UrlGenerationException` when parameters are missing is a
nice start, but knowing which parameters are missing can be a huge help
in figuring out where something is broken. Especially if a route has
multiple possible parameters. Which parameter is actually missing?
Are more than one missing? Now we'll know!
@simensen simensen force-pushed the missing-route-parameters-meaningful-messages branch from d387f35 to a24ccc2 Compare December 23, 2020 18:25
@simensen
Copy link
Contributor Author

@taylorotwell No problem. I fixed a styleci issue, rebased, and changed the base branch on GitHub. Looks like the tests are all queued again.

* @return static
*/
public static function forMissingParameters($route)
public static function forMissingParameters(Route $route, array $parameters = [])
Copy link
Member

Choose a reason for hiding this comment

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

We'll have to be aware that this is technically a breaking change for people extending this exception and method.

Copy link
Member

Choose a reason for hiding this comment

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

We could just re-target this PR at master. It's not long till another release anyway (indeed, it never really is, with the 6 month release cycle).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm happy with whatever. :)

@driesvints driesvints changed the title Give a more meaningul message when route parameters are missing [8.x] Give a more meaningul message when route parameters are missing Dec 23, 2020
Co-authored-by: Michel Bardelmeijer <m.bardelmeijer@gmail.com>
@taylorotwell taylorotwell merged commit 4da7a2b into laravel:8.x Jan 1, 2021
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

5 participants