Skip to content

Commit

Permalink
Log exceptions from run_task
Browse files Browse the repository at this point in the history
When a task was run from fork_task, any uncaught exceptions would
be silently discarded (e.g. a communication failure between worker
and hub). This makes debugging more difficult than it should be.

Make sure any raised exception is logged in this block.

Fixes release-engineering#32
  • Loading branch information
rohanpm committed Jan 4, 2019
1 parent e038778 commit 2b3ef33
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 0 deletions.
3 changes: 3 additions & 0 deletions kobo/worker/taskmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,9 @@ def fork_task(self, task_info):

# run the task
self.run_task(task_info)
except Exception:
self.log_critical("Error running forked task", exc_info=1)
# don't bother raising since we're about to exit
finally:
# die
os._exit(os.EX_OK)
Expand Down
29 changes: 29 additions & 0 deletions tests/test_taskmanager.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import errno
import os
import signal
import logging

import django
import pytest
Expand Down Expand Up @@ -750,6 +751,34 @@ def test_fork_task_runs_task_if_cant_fork(self):
t = Task.objects.get(id=t.id)
self.assertEqual(t.state, TASK_STATES['CLOSED'])

def test_fork_task_logs_exceptions(self):
"""Exceptions from the child within fork_task are logged."""

t = Task.objects.create(
worker=self._worker.worker,
arch=self._arch,
channel=self._channel,
owner=self._user,
method='DummyForegroundTask',
state=TASK_STATES['OPEN'],
)

logger = Mock()

with patch('kobo.worker.taskmanager.HubProxy') as hub_mock:
# Arrange for close_task call to fail (at end of task)
hub_mock.return_value.worker.close_task.side_effect = RuntimeError("simulated error")

tm = TaskManager(conf={'worker': self._worker}, logger=logger)
task_info = t.export(False)

with patch('kobo.worker.taskmanager.os', fork=Mock(return_value=0)) as os_mock:
os_mock.devnull = os.devnull
tm.fork_task(task_info)

# It should have logged something about the failure to close the task.
logger.log.assert_called_with(logging.CRITICAL, 'Error running forked task', exc_info=1)

@patch('kobo.worker.taskmanager.HubProxy', HubProxyMock)
def test_run_task_runs_foreground_task(self):
t = Task.objects.create(
Expand Down

0 comments on commit 2b3ef33

Please sign in to comment.