From 4c9c8e051479b6ca0d636889e70f1784d9703459 Mon Sep 17 00:00:00 2001 From: "patchback[bot]" <45432694+patchback[bot]@users.noreply.github.com> Date: Thu, 14 Jan 2021 14:40:33 +0100 Subject: [PATCH] npm - handle json decode exception (#1625) (#1636) * Provide a user friendly message by handling json decode exception rather than providing a stacktrace Fixes: #1614 Signed-off-by: Abhijeet Kasurde (cherry picked from commit a9c64655de4e75b573a49336e73a1487dbd29d0d) Co-authored-by: Abhijeet Kasurde --- changelogs/fragments/1614_npm.yml | 2 + plugins/modules/packaging/language/npm.py | 36 +++++----- .../modules/packaging/language/test_npm.py | 70 +++++++++++++++++++ 3 files changed, 92 insertions(+), 16 deletions(-) create mode 100644 changelogs/fragments/1614_npm.yml create mode 100644 tests/unit/plugins/modules/packaging/language/test_npm.py diff --git a/changelogs/fragments/1614_npm.yml b/changelogs/fragments/1614_npm.yml new file mode 100644 index 0000000000..0d39b1b6fb --- /dev/null +++ b/changelogs/fragments/1614_npm.yml @@ -0,0 +1,2 @@ +bugfixes: +- npm - handle json decode exception while parsing command line output (https://github.com/ansible-collections/community.general/issues/1614). diff --git a/plugins/modules/packaging/language/npm.py b/plugins/modules/packaging/language/npm.py index 42fe8d906c..3ef81eaa16 100644 --- a/plugins/modules/packaging/language/npm.py +++ b/plugins/modules/packaging/language/npm.py @@ -7,39 +7,39 @@ from __future__ import absolute_import, division, print_function __metaclass__ = type -DOCUMENTATION = ''' +DOCUMENTATION = r''' --- module: npm short_description: Manage node.js packages with npm description: - - Manage node.js packages with Node Package Manager (npm) + - Manage node.js packages with Node Package Manager (npm). author: "Chris Hoffman (@chrishoffman)" options: name: description: - - The name of a node.js library to install + - The name of a node.js library to install. type: str required: false path: description: - - The base path where to install the node.js libraries + - The base path where to install the node.js libraries. type: path required: false version: description: - - The version to be installed + - The version to be installed. type: str required: false global: description: - - Install the node.js library globally + - Install the node.js library globally. required: false default: no type: bool executable: description: - The executable location for npm. - - This is useful if you are using a version manager, such as nvm + - This is useful if you are using a version manager, such as nvm. type: path required: false ignore_scripts: @@ -55,12 +55,12 @@ options: default: no ci: description: - - Install packages based on package-lock file, same as running npm ci + - Install packages based on package-lock file, same as running C(npm ci). type: bool default: no production: description: - - Install dependencies in production mode, excluding devDependencies + - Install dependencies in production mode, excluding devDependencies. required: false type: bool default: no @@ -71,7 +71,7 @@ options: type: str state: description: - - The state of the node.js library + - The state of the node.js library. required: false type: str default: present @@ -80,7 +80,7 @@ requirements: - npm installed in bin path (recommended /usr/local/bin) ''' -EXAMPLES = ''' +EXAMPLES = r''' - name: Install "coffee-script" node.js package. community.general.npm: name: coffee-script @@ -124,12 +124,12 @@ EXAMPLES = ''' state: present ''' +import json import os import re from ansible.module_utils.basic import AnsibleModule - -import json +from ansible.module_utils._text import to_native class Npm(object): @@ -155,7 +155,7 @@ class Npm(object): else: self.name_version = self.name - def _exec(self, args, run_in_check_mode=False, check_rc=True): + def _exec(self, args, run_in_check_mode=False, check_rc=True, add_package_name=True): if not self.module.check_mode or (self.module.check_mode and run_in_check_mode): cmd = self.executable + args @@ -167,7 +167,7 @@ class Npm(object): cmd.append('--ignore-scripts') if self.unsafe_perm: cmd.append('--unsafe-perm') - if self.name: + if self.name and add_package_name: cmd.append(self.name_version) if self.registry: cmd.append('--registry') @@ -191,7 +191,11 @@ class Npm(object): installed = list() missing = list() - data = json.loads(self._exec(cmd, True, False)) + data = {} + try: + data = json.loads(self._exec(cmd, True, False, False) or '{}') + except (getattr(json, 'JSONDecodeError', ValueError)) as e: + self.module.fail_json(msg="Failed to parse NPM output with error %s" % to_native(e)) if 'dependencies' in data: for dep in data['dependencies']: if 'missing' in data['dependencies'][dep] and data['dependencies'][dep]['missing']: diff --git a/tests/unit/plugins/modules/packaging/language/test_npm.py b/tests/unit/plugins/modules/packaging/language/test_npm.py new file mode 100644 index 0000000000..849bfac1a6 --- /dev/null +++ b/tests/unit/plugins/modules/packaging/language/test_npm.py @@ -0,0 +1,70 @@ +# +# Copyright: (c) 2021, Abhijeet Kasurde +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +from ansible_collections.community.general.tests.unit.compat.mock import call, patch +from ansible_collections.community.general.plugins.modules.packaging.language import npm +from ansible_collections.community.general.tests.unit.plugins.modules.utils import ( + AnsibleExitJson, AnsibleFailJson, ModuleTestCase, set_module_args) + + +class NPMModuleTestCase(ModuleTestCase): + module = npm + + def setUp(self): + super(NPMModuleTestCase, self).setUp() + ansible_module_path = "ansible_collections.community.general.plugins.modules.packaging.language.npm.AnsibleModule" + self.mock_run_command = patch('%s.run_command' % ansible_module_path) + self.module_main_command = self.mock_run_command.start() + self.mock_get_bin_path = patch('%s.get_bin_path' % ansible_module_path) + self.get_bin_path = self.mock_get_bin_path.start() + self.get_bin_path.return_value = '/testbin/npm' + + def tearDown(self): + self.mock_run_command.stop() + self.mock_get_bin_path.stop() + super(NPMModuleTestCase, self).tearDown() + + def module_main(self, exit_exc): + with self.assertRaises(exit_exc) as exc: + self.module.main() + return exc.exception.args[0] + + def test_present(self): + set_module_args({ + 'name': 'coffee-script', + 'global': 'true', + 'state': 'present' + }) + self.module_main_command.side_effect = [ + (0, '{}', ''), + (0, '{}', ''), + ] + + result = self.module_main(AnsibleExitJson) + + self.assertTrue(result['changed']) + self.module_main_command.assert_has_calls([ + call(['/testbin/npm', 'list', '--json', '--long', '--global'], check_rc=False, cwd=None), + ]) + + def test_absent(self): + set_module_args({ + 'name': 'coffee-script', + 'global': 'true', + 'state': 'absent' + }) + self.module_main_command.side_effect = [ + (0, '{"dependencies": {"coffee-script": {}}}', ''), + (0, '{}', ''), + ] + + result = self.module_main(AnsibleExitJson) + + self.assertTrue(result['changed']) + self.module_main_command.assert_has_calls([ + call(['/testbin/npm', 'uninstall', '--global', 'coffee-script'], check_rc=True, cwd=None), + ])