diff options
author | Christopher Allan Webber <cwebber@dustycloud.org> | 2011-08-13 12:21:06 -0500 |
---|---|---|
committer | Christopher Allan Webber <cwebber@dustycloud.org> | 2011-08-13 12:21:06 -0500 |
commit | 6788b4123ef00241d6b6c80ca93d655e4307d6e3 (patch) | |
tree | 83510de6a4c8fc0852e539196c82c5020a5d58ed | |
parent | 4a477e246d07a4c26f084db2596caf3310b78609 (diff) | |
download | mediagoblin-6788b4123ef00241d6b6c80ca93d655e4307d6e3.tar.lz mediagoblin-6788b4123ef00241d6b6c80ca93d655e4307d6e3.tar.xz mediagoblin-6788b4123ef00241d6b6c80ca93d655e4307d6e3.zip |
Capture and properly handle errors.
Handled in several places:
- In the run() of the ProcessMedia itself for
handled (BaseProcessingFail derived) errors (best to do these not
in on_failure because the errors are highlighted in celeryd in a
way that looks inappropriate for when the errors are well handled)
- In ProcessMedia.on_failure() for all other errors
- In the submit view where all exceptions are caught, media is marked
at having failed, then the error is re-raised. (The reason for
this is that users running in "lazy" mode will get errors
propagated by celery and so on_failure won't run for them.)
-rw-r--r-- | mediagoblin/process_media/__init__.py | 54 | ||||
-rw-r--r-- | mediagoblin/submit/views.py | 21 |
2 files changed, 49 insertions, 26 deletions
diff --git a/mediagoblin/process_media/__init__.py b/mediagoblin/process_media/__init__.py index d6cdd747..69e4fc45 100644 --- a/mediagoblin/process_media/__init__.py +++ b/mediagoblin/process_media/__init__.py @@ -59,7 +59,14 @@ class ProcessMedia(Task): """ entry = mgg.database.MediaEntry.one( {'_id': ObjectId(media_id)}) - process_image(entry) + + # Try to process, and handle expected errors. + try: + process_image(entry) + except BaseProcessingFail, exc: + mark_entry_failed(entry[u'_id'], exc) + return + entry['state'] = u'processed' entry.save() @@ -71,31 +78,34 @@ class ProcessMedia(Task): we can use that to get more information about the failure and store that for conveying information to users about the failure, etc. """ - media_id = args[0] - entry = mgg.database.MediaEntry.one( - {'_id': ObjectId(media_id)}) + entry_id = args[0] + mark_entry_failed(entry_id, exc) - entry[u'state'] = u'failed' - - # Was this a BaseProcessingFail? In other words, was this a - # type of error that we know how to handle? - if isinstance(exc, BaseProcessingFail): - # Looks like yes, so record information about that failure and any - # metadata the user might have supplied. - entry[u'fail_error'] = exc.exception_path - entry[u'fail_metadata'] = exc.metadata - else: - # Looks like no, so just mark it as failed and don't record a - # failure_error (we'll assume it wasn't handled) and don't record - # metadata (in fact overwrite it if somehow it had previous info - # here) - entry[u'fail_error'] = None - entry[u'fail_metadata'] = {} - entry.save() +process_media = registry.tasks[ProcessMedia.name] -process_media = registry.tasks[ProcessMedia.name] +def mark_entry_failed(entry_id, exc): + # Was this a BaseProcessingFail? In other words, was this a + # type of error that we know how to handle? + if isinstance(exc, BaseProcessingFail): + # Looks like yes, so record information about that failure and any + # metadata the user might have supplied. + mgg.database['media_entries'].update( + {'_id': entry_id}, + {'$set': {u'state': u'failed', + u'fail_error': exc.exception_path, + u'fail_metadata': exc.metadata}}) + else: + # Looks like no, so just mark it as failed and don't record a + # failure_error (we'll assume it wasn't handled) and don't record + # metadata (in fact overwrite it if somehow it had previous info + # here) + mgg.database['media_entries'].update( + {'_id': entry_id}, + {'$set': {u'state': u'failed', + u'fail_error': None, + u'fail_metadata': {}}}) def process_image(entry): diff --git a/mediagoblin/submit/views.py b/mediagoblin/submit/views.py index 25b3664b..1ba17954 100644 --- a/mediagoblin/submit/views.py +++ b/mediagoblin/submit/views.py @@ -28,7 +28,7 @@ from mediagoblin.util import ( from mediagoblin.util import pass_to_ugettext as _ from mediagoblin.decorators import require_active_login from mediagoblin.submit import forms as submit_forms, security -from mediagoblin.process_media import process_media +from mediagoblin.process_media import process_media, mark_entry_failed from mediagoblin.messages import add_message, SUCCESS @@ -102,9 +102,22 @@ def submit_start(request): # # (... don't change entry after this point to avoid race # conditions with changes to the document via processing code) - process_media.apply_async( - [unicode(entry['_id'])], {}, - task_id=task_id) + try: + process_media.apply_async( + [unicode(entry['_id'])], {}, + task_id=task_id) + except BaseException as exc: + # The purpose of this section is because when running in "lazy" + # or always-eager-with-exceptions-propagated celery mode that + # the failure handling won't happen on Celery end. Since we + # expect a lot of users to run things in this way we have to + # capture stuff here. + # + # ... not completely the diaper pattern because the exception is + # re-raised :) + mark_entry_failed(entry[u'_id'], exc) + # re-raise the exception + raise add_message(request, SUCCESS, _('Woohoo! Submitted!')) |