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

Output json_last_error_msg instead of json_last_error #1045

Merged
merged 1 commit into from
Jul 16, 2020

Conversation

jkrzefski
Copy link
Contributor

The JsonErrorException already converts the error code from json_last_error into a human readable error message. The RuntimeException however does not do this. To have more consistency between the two and for improved human readability I have changed json_last_error to json_last_error_msg. Error messages now no longer look like this:

[Elasticsearch\Common\Exceptions\RuntimeException]  
Failed to JSON encode: 5

And instead they look more like this:

[Elasticsearch\Common\Exceptions\RuntimeException]                               
Failed to JSON encode: Malformed UTF-8 characters, possibly incorrectly encoded

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@cla-checker-service
Copy link

cla-checker-service bot commented Jun 25, 2020

💚 CLA has been signed

@jkrzefski
Copy link
Contributor Author

I signed the CLA now. How can I trigger the CLA checker service again?

@ezimuel
Copy link
Contributor

ezimuel commented Jul 16, 2020

@jkrzefski thanks for this PR. I checked and the CLA is ok. I'm checking about the Travis CI failing. The PR seems good to me.

@ezimuel ezimuel added this to the 7.9.0 milestone Jul 16, 2020
@ezimuel
Copy link
Contributor

ezimuel commented Jul 16, 2020

@jkrzefski I see the issue. You need to send the PR against 7.x and not master. I suggest to rebase from 7.x and send a new PR. Thanks!

I just added a Contributing section about that in the README.

@jkrzefski jkrzefski changed the base branch from master to 7.x July 16, 2020 09:01
@jkrzefski
Copy link
Contributor Author

@ezimuel I rebased my changes and changed the target branch. Will I have to file a new pull request or is this also ok?

@ezimuel
Copy link
Contributor

ezimuel commented Jul 16, 2020

@jkrzefski thanks everything is ok now.

@ezimuel ezimuel merged commit a90148a into elastic:7.x Jul 16, 2020
@jkrzefski jkrzefski deleted the patch-1 branch July 16, 2020 11:39
@ezimuel
Copy link
Contributor

ezimuel commented Aug 18, 2020

Just released elasticsearch-php 7.9.0 with #1045 included.

@ezimuel ezimuel mentioned this pull request Aug 13, 2021
4 tasks
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.

3 participants