Better exception handling

[subhankarb]

We have very generic exception and in some places we are even swallowing them.

Acceptance criteria

• No exceptions swallowed silently
• Specific exceptions were possible

Tasks

• Got through code base and list out all exceptions that are silently swallowed
• Use specific exception in app/init.py
• Change tests.

Analysis

swallowing exceptions

• get_s3_object() - https://github.com/frictionlessdata/dpr-api/blob/master/app/package/models.py#L69
• delete_data_package() - https://github.com/frictionlessdata/dpr-api/blob/master/app/package/models.py#L177
• change_acl - https://github.com/frictionlessdata/dpr-api/blob/master/app/package/models.py#L205
• copy_to_new_version() - https://github.com/frictionlessdata/dpr-api/blob/master/app/package/models.py#L228
• create_or_update_tag() - https://github.com/frictionlessdata/dpr-api/blob/master/app/package/models.py#L294
• change_status() - https://github.com/frictionlessdata/dpr-api/blob/master/app/package/models.py#L346
• delete_data_package() - https://github.com/frictionlessdata/dpr-api/blob/master/app/package/models.py#L368
• get_package() - https://github.com/frictionlessdata/dpr-api/blob/master/app/package/models.py#L385
• datapackage_show() - https://github.com/frictionlessdata/dpr-api/blob/master/app/site/controllers.py#L58

Generic exceptions

• get_user_from_cookie() - https://github.com/frictionlessdata/dpr-api/blob/master/app/__init__.py#L120.
• tag_data_package() https://github.com/frictionlessdata/dpr-api/blob/master/app/package/controllers.py#L86
• delete_data_package() - https://github.com/frictionlessdata/dpr-api/blob/master/app/package/controllers.py#L140
• undelete_data_package() - https://github.com/frictionlessdata/dpr-api/blob/master/app/package/controllers.py#L194
• purge_data_package() - https://github.com/frictionlessdata/dpr-api/blob/master/app/package/controllers.py#L247

Other comments

More specific exception handling, general Exception is always bad, should be specific type of exception.

• https://github.com/frictionlessdata/dpr-api/blob/master/app/__init__.py#L120.

On the above two points, and an earlier one - If we handle Exception and return False, especially on operations such as reading, writing and deleting user data, we have a real problem waiting to appear in unexpected ways.

• https://github.com/frictionlessdata/dpr-api/blob/master/app/package/models.py#L228

This seems like a bad place to silently swallow an exception:

• https://github.com/frictionlessdata/dpr-api/blob/master/app/package/models.py#L177
• https://github.com/frictionlessdata/dpr-api/blob/master/app/package/models.py#L204

[rufuspollock]

@zelima one comment - i don't mind using generic exception object Exception as long as we have a good message (trying to create specific types of exception as classes is usually pointless at this stage -- it only matters if you are later trying to catch only a specific type of exception).

[zelima]

FIXED