Added required error message.

[vinogcs]

If any JSON error occurs user will be be shown error message if the app mode is not in Production.

Description (*)

Added error description in JSON serialization.

Fixed Issues (if relevant)

Manual testing scenarios (*)

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds on Travis CI are green)

[magento-engcom-team]

Hi @vinogcs. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento-engcom-team give me test instance - deploy test instance based on PR changes
• @magento-engcom-team give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

[magento-cicd2]

All committers have signed the CLA.

[sivaschenko]

Hi @vinogcs thanks for your contribution! In my opinion the error message should be the same for production and dev environments. For production/frontend such errors should be properly handled on the higher layer catching that exception and adding a message that is understandable for user (as I think both error message and added description are not user-friendly).

[vinogcs]

Hi @sivaschenko, I agree to your point that it has to be taken care by developers at their end and the error message I've added are not clearly understandable. But many non-technical people are adopting Magento for their E-Commerce and non E-Commerce application today. There are few places where these are directly shown to end users who don't have any idea what the error is.

Yesterday, one of my MIS team tried to import products from CSV, they got this error and had no idea what the actual problem was and internet and stack overflow failed when they searched with the error message "Unable to serialize value".

So I thought it could be better to have little more description on error. The error I've added will tell that the problem was with input file, if it is(in my case it was malformed UTF characters). And I think it will also help developers to include better response to their exception management without worrying about validating input file separately.

Maybe I'm wrong, it's just my 2 cents.. :)

[sivaschenko]

@vinogcs thanks for details. I completely agree with your point of view! I meant that I don't think isOnProduction check is necessary. Considering that the error message is anyway technical, I'd output the detailed error message on both dev and production environments.

If you don't think it's appropriate to output all the details on production environments I'd suggest to dump the details to the log.

Please let me know what do you think about it.

[vinogcs]

@sivaschenko I think you're right. It would consume more resources to check the state of the app just to show the error message. We can just go for showing error irrespective of the environment.

[sivaschenko]

Hi @vinogcs , thanks for your update! Please consider removing of added trailing spaces as per my suggestio

[vinogcs]

Oops!. My Bad.

[magento-engcom-team]

Hi @sivaschenko, thank you for the review.
ENGCOM-3782 has been created to process this Pull Request

[magento-engcom-team]

@vinogcs thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

[ghost]

Hi @vinogcs, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

[magento-engcom-team]

Hi @vinogcs. Thank you for your contribution.
We will aim to release these changes as part of 2.3.1.
Please check the release notes for final confirmation.