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

Improve Exception handling for 422 / existing items #170

Closed
cord opened this issue Oct 30, 2023 · 8 comments
Closed

Improve Exception handling for 422 / existing items #170

cord opened this issue Oct 30, 2023 · 8 comments

Comments

@cord
Copy link

cord commented Oct 30, 2023

requests like

$forge→createDatabase('123', [
'name' => 'test'
'user' => 'dojziam11j',
'password' => 'hQUHWg3ovi'
]);

can trigger a misleading Exception

Laravel\Forge\Exceptions\ValidationException
The given data failed to pass validation.

Testing in postman reveals the true reason:

CleanShot 2023-10-30 at 12 38 53

I would expect to receive a meaningful exception with more information about the cause.

@driesvints
Copy link
Member

We’d welcome a pr to make this more clearer

@cord
Copy link
Author

cord commented Oct 30, 2023

Is this the official API package of the commercial forge business?

@jbrooksuk
Copy link
Member

@cord this is the official package, yes.

@cord
Copy link
Author

cord commented Oct 31, 2023

Great, would be nice if you can improve the customer experience with the API package by returning a meaningful exception in case of already existing items.

@jbrooksuk
Copy link
Member

@cord, you can do the following:

use Laravel\Forge\Exceptions\ValidationException;

try {
  $forge→createDatabase('123', [
    'name' => 'test'
    'user' => 'dojziam11j',
    'password' => 'hQUHWg3ovi'
  ]);
} catch (ValidationException $e) {
  dump($e->errors());
}

@cord
Copy link
Author

cord commented Nov 1, 2023

@jbrooksuk thanks, that returns some information.

However I would expect different types of exception based on situation. Probably requires some work on forge as well.

@jbrooksuk
Copy link
Member

@cord what kind of exceptions are you expecting? The error is a validation error, so a ValidationException makes sense to me.

@cord
Copy link
Author

cord commented Nov 1, 2023

@jbrooksuk it seems ValidationException is used to cover any 422 situation, regardless of the reason. If the code should react on the different situations (e.g. 'name already taken' , 'invalid name') we would need to compare the returned strings.

When trying to create a server with a taken name a FailedActionException is thrown, so different approach here.

So I wish there would be different exceptions for each situation we need to handle.

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

No branches or pull requests

3 participants