LaraBug / LaraBug

Laravel error reporting tool
https://www.larabug.com
MIT License
267 stars 58 forks source link

Fix 422 error when sending Exceptions with no message #73

Closed pauloimon closed 1 year ago

pauloimon commented 2 years ago

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 commented 2 years ago

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

@Glennmen @SebastiaanKloos What do you think?

Glennmen commented 2 years ago

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

pauloimon commented 2 years ago

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.

Cannonb4ll commented 1 year ago

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

SebastiaanKloos commented 1 year ago

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

Yes! Thanks @pauloimon 🙏