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

Improved text of exception message in case of error in module's composer.json #9165

Merged
merged 3 commits into from
Apr 13, 2017

Conversation

vovayatsyuk
Copy link
Member

@vovayatsyuk vovayatsyuk commented Apr 7, 2017

Description

When some third-party module has syntax error inside its composer.json, it's very hard to determine which
module caused an exception.

This PR changes default message with the following:

Some_Module's composer.json error: Decoding failed: Syntax error

where Decoding failed: Syntax error - is a message of the original exception

Manual testing scenarios

Sample data must be installed to test this behavior.

  1. Make an error in app/code/Magento/Cms/composer.json:

  2. Run php bin/magento sampeladata:reset. You'll receive a message:

    [Zend_Json_Exception]          
    Decoding failed: Syntax error
    

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)

When some third-party module has syntax error inside its composer.json, it's very hard to determine which
module caused an exception. 

This PR changes default message with the following:

```
Some_Module's composer.json error: Decoding failed: Syntax error
```
@vovayatsyuk vovayatsyuk changed the title Improved text of exception message Improved text of exception message in case of error in module's composer.json Apr 7, 2017
@adragus-inviqa
Copy link
Contributor

I wouldn't concat any other string with the module name; it'd keep it completely separate, to make it more obvious.
Something likeError decoding composer.json for module Some_Module.

@vovayatsyuk
Copy link
Member Author

I'd like to keep original exception message without change, because there are multiple possible variants for it. So we need a template that will suite for all kind of messages.

- Original exception message moved into 'sprintf' arguments
- "'s" suffix removed from module name
@adragus-inviqa
Copy link
Contributor

I'm not sure what you mean by "a template that will suite for all kind of messages"; it just has to be a good format.

But your recent change looks good. I just didn't like the interpolation: $moduleName's.

@okorshenko okorshenko self-assigned this Apr 12, 2017
@okorshenko okorshenko added this to the April 2017 milestone Apr 12, 2017
@magento-team magento-team merged commit c22243e into magento:develop Apr 13, 2017
magento-team pushed a commit that referenced this pull request Apr 13, 2017
magento-team pushed a commit that referenced this pull request Apr 13, 2017
@magento-team
Copy link
Contributor

@vovayatsyuk thank you for your contribution. Your Pull Request has been successfully merged

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