aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorChristopher Allan Webber <cwebber@dustycloud.org>2011-08-13 12:21:06 -0500
committerChristopher Allan Webber <cwebber@dustycloud.org>2011-08-13 12:21:06 -0500
commit6788b4123ef00241d6b6c80ca93d655e4307d6e3 (patch)
tree83510de6a4c8fc0852e539196c82c5020a5d58ed
parent4a477e246d07a4c26f084db2596caf3310b78609 (diff)
downloadmediagoblin-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__.py54
-rw-r--r--mediagoblin/submit/views.py21
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!'))