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

[5.4] Improve Queue's InvalidPayloadException error message #20143

Merged
merged 2 commits into from
Jul 19, 2017
Merged

[5.4] Improve Queue's InvalidPayloadException error message #20143

merged 2 commits into from
Jul 19, 2017

Conversation

timacdonald
Copy link
Member

Currently the queue can throw an InvalidPayloadException with just the last_json_error() number in the message.

screen shot 2017-07-19 at 1 33 45 pm

I had to jump into the Queue source to understand what was actually the issue happening here. I thought perhaps we could simply give a little more context to the error. The message itself may not be exactly what should be in there - suggestions welcomed - but I think it would be good to present more info. The suggested message is:

screen shot 2017-07-19 at 1 56 53 pm

Maybe this is more obvious to more experienced dev's then myself and isn't needed.

@lucasmichot
Copy link
Contributor

Why not just setting the default in \Illuminate\Queue\InvalidPayloadException::__construct instead?

@timacdonald
Copy link
Member Author

I considered that also, and the exception is only used in the one place at the moment. Just wasn't sure if that is a suitable default error message for that exception or not - as the exception is not specifically a JSON error exception. But I guess that can just be changed in the future if a non-json related error occurs that throws this exception.

@taylorotwell taylorotwell merged commit 7107e57 into laravel:5.4 Jul 19, 2017
@BrandonSurowiec
Copy link
Contributor

Would it be more informative to use 'json_last_error_msg() instead of, or in addition to the error code?

@timacdonald timacdonald deleted the improve-exception-message branch July 19, 2017 23:02
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.

4 participants