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

Fix 422 error when sending Exceptions with no message #73

Merged
merged 2 commits into from
Jun 30, 2022
Merged

Fix 422 error when sending Exceptions with no message #73

merged 2 commits into from
Jun 30, 2022

Conversation

pauloimon
Copy link
Contributor

@pauloimon pauloimon commented Apr 25, 2022

Sometimes, when the package is sending Exceptions to LaraBug API, it returns the HTTP error 422 due to empty message on them.

Steps to reproduce

Just throw an Exception with no message on anywhere in your code:

throw new \Exception();

And now capture the error on line 94/95 of ./src/LaraBug.php file like this:

$rawResponse = $this->logError($data);
dd($rawResponse->getBody()->getContents());

The LaraBug API will return:

{ "error": "Did not receive the correct parameters to process this exception" }

Solution

I added a Null Coalescing operator on getExceptionData() to fill the Exception's message with a default value if not exists.

@Cannonb4ll
Copy link
Contributor

I'm not sure we want to accept exceptions without exceptions(?).

@Glennmen @SebastiaanKloos What do you think?

@Glennmen
Copy link
Collaborator

Could you give an example when this would happen 🤔 I don't see any reason why it would be empty.

@pauloimon
Copy link
Contributor Author

I agree with you. Exceptions must always have a message.

But I found it when I was testing the package and to me it seemed make no sense ignore them because of that.

I know that the messages are used as log titles on LaraBug Dashboard, but why we can't add a default value instead of ignore them?

A better approach would be use the exception types as default value:

$data['exception'] = $exception->getMessage() ?? get_class($exception);

You can close this PR if this make no sense to LaraBug business rules.

src/LaraBug.php Outdated
@@ -155,7 +155,7 @@ public function getExceptionData(Throwable $exception)
$data['host'] = Request::server('SERVER_NAME');
$data['method'] = Request::method();
$data['fullUrl'] = Request::fullUrl();
$data['exception'] = $exception->getMessage();
$data['exception'] = $exception->getMessage() ?? 'Exception with no message';
Copy link
Contributor

Choose a reason for hiding this comment

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

I would suggest changing this to '-' or something

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed!

@Cannonb4ll
Copy link
Contributor

@SebastiaanKloos I think this one is ready to be merged right?

@SebastiaanKloos
Copy link
Contributor

@SebastiaanKloos I think this one is ready to be merged right?

Yes! Thanks @pauloimon 🙏

@SebastiaanKloos SebastiaanKloos merged commit e1609ee into LaraBug:master Jun 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants