~sirn/fanboi2

8b56ac65160b4b491c3a1bf0864cb47f9e996cd1 — Kridsada Thanabulpong 7 years ago 630b0bc
Fix broken error report due to Celery no longer raise original exceptions.

Celery changed from raising the original exception class to raising the
proxy class inside celery.backends.base and therefore our error reporting
which heavily relies on exceptions no longer working.

This commit changes so that tasks always success even though the post
could not be posted and detect rejection through return codes instead.
M CHANGES.rst => CHANGES.rst +11 -1
@@ 1,6 1,16 @@
0.8.1
0.8.3
=====

- Changed how post processing failures are handle to no longer rely on `Celery <http://www.celeryproject.org>`_'s exceptions.

0.8.2
-----

- Fixed Akismet never timed out causing post to hang forever.

0.8.1
-----

- Fixed broken debug toolbar in development mode due to Python 3.2.3 bug.
- Removed pyramid_zcml and webtest from application requirements.
- Changed Celery process runner to no longer load Pyramid environment.

M fanboi2/tasks.py => fanboi2/tasks.py +8 -20
@@ 20,22 20,14 @@ def configure_celery(settings):  # pragma: no cover
    }


class TaskException(Exception):
    pass


class AddTopicException(TaskException):
    pass


@celery.task(throws=(AddTopicException,))
@celery.task()
def add_topic(request, board_id, title, body):
    """Insert a topic to the database."""
    if akismet.spam(request, body):
        raise AddTopicException('spam')
        return 'failure', 'spam'

    if dnsbl.listed(request['remote_addr']):
        raise AddTopicException('dnsbl')
        return 'failure', 'dnsbl'

    with transaction.manager:
        board = DBSession.query(Board).get(board_id)


@@ 46,24 38,20 @@ def add_topic(request, board_id, title, body):
        return 'topic', post.topic_id


class AddPostException(TaskException):
    pass


@celery.task(bind=True, throws=(AddPostException,), max_retries=4)  # 5 total.
@celery.task(bind=True, max_retries=4)  # 5 total.
def add_post(self, request, topic_id, body, bumped):
    """Insert a post to a topic."""
    if akismet.spam(request, body):
        raise AddPostException('spam')
        return 'failure', 'spam'

    with transaction.manager:
        topic = DBSession.query(Topic).get(topic_id)
        if topic.status != "open":
            return 'failure', topic.status

        post = Post(topic=topic, body=body, bumped=bumped)
        post.ip_address = request['remote_addr']

        if topic.status != "open":
            raise AddPostException(topic.status)

        try:
            DBSession.add(post)
            DBSession.flush()

M fanboi2/templates/boards/error.jinja2 => fanboi2/templates/boards/error.jinja2 +4 -0
@@ 19,6 19,10 @@
                        <h3>I'm sorry, Dave. I'm afraid I can't do that.</h3>
                        <p>This mission is too important for me to allow you to jeopardize it.</p>
                        <p class="fineprint">Your message has been identified as spam and therefore rejected.</p>
                    {% elif error == "dnsbl" %}
                        <h3>Rainbows! Rainbows everywhere!</h3>
                        <p>What about us finding the beginning of the rainbow together.</p>
                        <p class="fineprint">Your IP address is listed in DNSBL and therefore rejected.</p>
                    {% endif %}
                {% endif %}
            </div>

M fanboi2/templates/topics/error.jinja2 => fanboi2/templates/topics/error.jinja2 +0 -4
@@ 19,10 19,6 @@
                        <h3>I'm sorry, Dave. I'm afraid I can't do that.</h3>
                        <p>This mission is too important for me to allow you to jeopardize it.</p>
                        <p class="fineprint">Your message has been identified as spam and therefore rejected.</p>
                    {% elif error == "dnsbl" %}
                        <h3>Rainbows! Rainbows everywhere!</h3>
                        <p>What about us finding the beginning of the rainbow together.</p>
                        <p class="fineprint">Your IP address is listed in one of DNS providers and therefore rejected.</p>
                    {% elif error == "archived" %}
                        <h3>Post limit exceeded</h3>
                        <p>Please start a new topic.</p>

M fanboi2/tests/test_tasks.py => fanboi2/tests/test_tasks.py +10 -20
@@ 25,10 25,10 @@ class TestAddTopicTask(TaskMixin, ModelMixin, unittest.TestCase):
        self.assertEqual(DBSession.query(Topic).get(result.get()[1]), topic)
        self.assertEqual(topic.title, 'Foobar')
        self.assertEqual(topic.posts[0].body, 'Hello, world!')
        self.assertEqual(result.result, ('topic', topic.id))

    @mock.patch('fanboi2.utils.Akismet.spam')
    def test_add_topic_spam(self, akismet):
        from fanboi2.tasks import AddTopicException
        from fanboi2.models import Topic
        akismet.return_value = True
        request = {'remote_addr': '127.0.0.1'}


@@ 36,15 36,12 @@ class TestAddTopicTask(TaskMixin, ModelMixin, unittest.TestCase):
            board = self._makeBoard(title='Foobar', slug='foobar')
            board_id = board.id  # board is not bound outside transaction!
        result = self._makeOne(request, board_id, 'Foobar', 'Hello, world!')
        self.assertFalse(result.successful())
        self.assertTrue(result.successful())
        self.assertEqual(DBSession.query(Topic).count(), 0)
        with self.assertRaises(AddTopicException) as e:
            assert not result.get()
        self.assertEqual(e.exception.args, ('spam',))
        self.assertEqual(result.result, ('failure', 'spam'))

    @mock.patch('fanboi2.utils.Dnsbl.listed')
    def test_add_topic_dnsbl(self, dnsbl):
        from fanboi2.tasks import AddTopicException
        from fanboi2.models import Topic
        dnsbl.return_value = True
        request = {'remote_addr': '127.0.0.1'}


@@ 52,11 49,9 @@ class TestAddTopicTask(TaskMixin, ModelMixin, unittest.TestCase):
            board = self._makeBoard(title='Foobar', slug='foobar')
            board_id = board.id  # board is not bound outside transaction!
        result = self._makeOne(request, board_id, 'Foobar', 'Hello, world!')
        self.assertFalse(result.successful())
        self.assertTrue(result.successful())
        self.assertEqual(DBSession.query(Topic).count(), 0)
        with self.assertRaises(AddTopicException) as e:
            assert not result.get()
        self.assertEqual(e.exception.args, ('dnsbl',))
        self.assertEqual(result.result, ('failure', 'dnsbl'))


class TestAddPostTask(TaskMixin, ModelMixin, unittest.TestCase):


@@ 80,11 75,11 @@ class TestAddPostTask(TaskMixin, ModelMixin, unittest.TestCase):
        self.assertEqual(DBSession.query(Post).get(result.get()[1]), post)
        self.assertEqual(post.body, 'Hi!')
        self.assertEqual(post.bumped, True)
        self.assertEqual(result.result, ('post', post.id))

    @mock.patch('fanboi2.utils.Akismet.spam')
    def test_add_post_spam(self, akismet):
        import transaction
        from fanboi2.tasks import AddPostException
        from fanboi2.models import Post
        akismet.return_value = True
        request = {'remote_addr': '127.0.0.1'}


@@ 93,15 88,12 @@ class TestAddPostTask(TaskMixin, ModelMixin, unittest.TestCase):
            topic = self._makeTopic(board=board, title='Hello, world!')
            topic_id = topic.id  # topic is not bound outside transaction!
        result = self._makeOne(request, topic_id, 'Hi!', True)
        self.assertFalse(result.successful())
        self.assertTrue(result.successful())
        self.assertEqual(DBSession.query(Post).count(), 0)
        with self.assertRaises(AddPostException) as e:
            assert not result.get()
        self.assertEqual(e.exception.args, ('spam',))
        self.assertEqual(result.result, ('failure', 'spam'))

    def test_add_post_locked(self):
        import transaction
        from fanboi2.tasks import AddPostException
        from fanboi2.models import Post
        request = {'remote_addr': '127.0.0.1'}
        with transaction.manager:


@@ 112,11 104,9 @@ class TestAddPostTask(TaskMixin, ModelMixin, unittest.TestCase):
                status='locked')
            topic_id = topic.id  # topic is not bound outside transaction!
        result = self._makeOne(request, topic_id, 'Hi!', True)
        self.assertFalse(result.successful())
        self.assertTrue(result.successful())
        self.assertEqual(DBSession.query(Post).count(), 0)
        with self.assertRaises(AddPostException) as e:
            assert not result.get()
        self.assertEqual(e.exception.args, ('locked',))
        self.assertEqual(result.result, ('failure', 'locked'))

    def test_add_post_retry(self):
        import transaction

M fanboi2/tests/test_views.py => fanboi2/tests/test_views.py +6 -12
@@ 138,10 138,8 @@ class TestBaseTaskView(ViewMixin, ModelMixin, unittest.TestCase):

    @mock.patch('fanboi2.tasks.celery.AsyncResult')
    def test_get_dispatch_failure(self, async):
        from celery.states import FAILURE
        from fanboi2.tasks import TaskException
        exception = TaskException('foobar')
        async.return_value = DummyAsyncResult(FAILURE, None, exception)
        from celery.states import SUCCESS
        async.return_value = DummyAsyncResult(SUCCESS, ('failure', 'foobar'))
        request = self._GET({'task': '1234'})
        view = self._makeOne()(request)
        with mock.patch.object(view, 'GET_failure') as get_call:


@@ 291,10 289,8 @@ class TestBoardNewView(ViewMixin, ModelMixin, unittest.TestCase):

    @mock.patch('fanboi2.tasks.celery.AsyncResult')
    def test_get_failure(self, async):
        from celery.states import FAILURE
        from fanboi2.tasks import TaskException
        exception = TaskException('foobar')
        async.return_value = DummyAsyncResult(FAILURE, None, exception)
        from celery.states import SUCCESS
        async.return_value = DummyAsyncResult(SUCCESS, ('failure', 'foobar'))
        self.config.testing_add_renderer('boards/error.jinja2')
        request = self._GET({'task': '1234'})
        response = self._getTargetClass()(request)()


@@ 436,10 432,8 @@ class TestTopicView(ViewMixin, ModelMixin, unittest.TestCase):

    @mock.patch('fanboi2.tasks.celery.AsyncResult')
    def test_get_failure(self, async):
        from celery.states import FAILURE
        from fanboi2.tasks import TaskException
        exception = TaskException('foobar')
        async.return_value = DummyAsyncResult(FAILURE, None, exception)
        from celery.states import SUCCESS
        async.return_value = DummyAsyncResult(SUCCESS, ('failure', 'foobar'))
        self.config.testing_add_renderer('topics/error.jinja2')
        request = self._GET({'task': '1234'})
        response = self._getTargetClass()(request)()

M fanboi2/views.py => fanboi2/views.py +7 -9
@@ 8,7 8,7 @@ from sqlalchemy.orm import undefer
from sqlalchemy.orm.exc import NoResultFound
from .forms import TopicForm, PostForm
from .models import Topic, Post, Board, DBSession
from .tasks import celery, add_topic, add_post, TaskException
from .tasks import celery, add_topic, add_post
from .utils import RateLimiter, serialize_request




@@ 74,14 74,12 @@ class BaseTaskView(object):
            task = celery.AsyncResult(self.request.params['task'])

            if task.state == states.SUCCESS:
                obj = self._serialize(task.get())
                return self.GET_success(obj)

            elif task.state == states.FAILURE:
                try:
                    task.get()
                except TaskException as exc:
                    return self.GET_failure(exc.args[0])
                result = task.get()
                if result[0] == 'failure':
                    return self.GET_failure(result[1])
                else:
                    obj = self._serialize(result)
                    return self.GET_success(obj)

            else:
                return self.GET_task()

M setup.py => setup.py +1 -1
@@ 47,7 47,7 @@ requires = [
    ]

setup(name='fanboi2',
      version='0.8.2',
      version='0.8.3',
      description='fanboi2',
      long_description=readme + '\n\n' + changes,
      classifiers=[