Skip to content

Commit

Permalink
[fix] CommandFailedException: ensure error message is always present
Browse files Browse the repository at this point in the history
If a command with suppressed output failed, CommandFailedException
would be raised with an emptry string as argument, which makes
debugging issues really hard.

In this cases we shall instantiate the exception with the same
message passed to the log.
  • Loading branch information
nemesifier committed Aug 16, 2021
1 parent cd55ea4 commit 05a3d7d
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 2 deletions.
6 changes: 4 additions & 2 deletions openwisp_controller/connection/connectors/ssh.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,9 +170,11 @@ def exec_command(
# abort the operation if any of the command
# returned with a non-zero exit status
if exit_status not in exit_codes and raise_unexpected_exit:
logger.error('Unexpected exit code: {0}'.format(exit_status))
log_message = 'Unexpected exit code: {0}'.format(exit_status)
logger.error(log_message)
message = error if error else output
raise CommandFailedException(message)
# if message is empty, use log_message
raise CommandFailedException(message or log_message)
return output, exit_status

def update_config(self): # pragma: no cover
Expand Down
16 changes: 16 additions & 0 deletions openwisp_controller/connection/tests/test_ssh.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,23 @@ def test_connection_failed_command(self, mocked_debug, mocked_info):
mock.call('/bin/sh: 1: wrongcommand: not found'),
mock.call('Unexpected exit code: 127'),
]

@mock.patch.object(ssh_logger, 'info')
@mock.patch.object(ssh_logger, 'debug')
def test_connection_failed_command_suppressed_output(
self, mocked_debug, mocked_info
):
ckey = self._create_credentials_with_key(port=self.ssh_server.port)
dc = self._create_device_connection(credentials=ckey)
dc.connector_instance.connect()
with self.assertRaises(Exception) as ctx:
with mock.patch('logging.Logger.error') as mocked_logger:
dc.connector_instance.exec_command(
'rm /thisfilesurelydoesnotexist 2> /dev/null'
)
log_message = 'Unexpected exit code: 1'
mocked_logger.assert_has_calls([mock.call(log_message)])
self.assertEqual(str(ctx.exception), log_message)

@mock.patch('scp.SCPClient.putfo')
def test_connection_upload(self, putfo_mocked):
Expand Down

0 comments on commit 05a3d7d

Please sign in to comment.