Possibility to raise more specific exception when the table does not exist

[huyphan]

I can contribute PR for this but I'm leaving the issue here first to collect some thoughts.

Currently if we skip creating table when binding a model and the table does not exist yet, loading objects will result generic exception bloop.exceptions.BloopException:

In [7]: from bloop import BaseModel, String, Engine
   ...:
   ...:
   ...: class UserModel(BaseModel):
   ...:     id = Column(String, hash_key=True)
   ...:     name = Column(String)
   ...:
   ...: engine = Engine()
   ...: engine.bind(UserModel, skip_table_setup=True)
   ...:
   ...: try:
   ...:     model = UserModel(id="Foo")
   ...:     engine.load(model, consistent=True)
   ...: except Exception as ex:
   ...:     print(f"Exception type: {type(ex)}")
   ...:     print(f"Exception message: {ex}")
   ...:

Exception type: <class 'bloop.exceptions.BloopException'>
Exception message: Unexpected error while loading items.

I enabled debug log and found the underlying exception returned by DynamoDB:

2020-01-14 04:13:08,399 [DEBUG] Response body:
b'{"__type":"com.amazonaws.dynamodb.v20120810#ResourceNotFoundException","message":"Requested resource not found"}'

For get_item/batch_get_item API, I think it's safe to assume that ResourceNotFoundException is only returned when the table does not exist. Hence we should be able to raise a specific exception here.

The issue with that assumption is that it might not be future-proof. So one safe option is to still return a generic exception, but less generic than BloopException -- I'm thinking of MissingResource so it's consistent with existing MIssingObjects exception.

[numberoverzero]

Thanks for opening an issue, and the great repro steps! I think you've identified an opportunity to simplify error handling, although I'm not sold on an additional subclass.

Existing Workaround

The exception is raised in SessionWrapper.load_items which should preserve the original exception. As a first step, you can access this from ex.__cause__:

The expression following from must be an exception or None. It will be set as __cause__ on the raised exception.

The code to check the error message is a little bit tedious since botocore exceptions are afaik just json blobs wrapped in a ClientError class. This looks like:

except bloop.BloopException as ex:
    code = ex.__cause__.response["Error"]["Code"]
    if code == "ResourceNotFoundException":  # <-- string literal TBD
        # handle missing resource

Improving BloopException

Long term I'd like to avoid re-wrapping the inner response code as a new subclass of BloopException. Raising up the exception codes to individual classes adds mental overhead when debugging, a synchronicity problem when new codes are created, and complexity when seeking help on the aws forums or this issue tracker.

However, easy access to the error code seems like a great enhancement. Something like BloopException.boto_error_code as an Optional[str] would simplify common error handling without requiring a new version every time the Dynamo team adds a new error code.

What are your thoughts?

[huyphan]

It makes sense regarding the overhead of having explicit exception class for each error code.

I did not realize that we can access the underlying error code via __cause__ attribute. I'm not a big fan of dunder attribute usage, so it would be cool if we can access the error code in a nicer way -- and your suggestion on BloopException.boto_error_code is very reasonable.

---

Another improvement that maybe easier to add, is making the underlying error code part of string representation of BloopException. So instead of just returning this:

>>> str(ex)
Unexpected error while loading items

... it could return this:

>>> str(ex)
Unexpected error while loading items: ResourceNotFoundException.

This improvement does not add much value to error handling though, it just makes the (unexpected) errors more informative.

[numberoverzero]

For now I'll merge a PR that improves the error string, if you're up for making the change.

I'd like to think about the larger BloopException change for a few days before making such a large change. That's the kind of structure people build error handling around, and as such is not easy to back out even in a major version bump.