Better Exceptions for Vigilant Crypto Snatch

Code & Zahlen

When running the Vigilant Crypto Snatch software, I often get these error messages sent via Telegram:

  • 🔴 An exception of type <class 'vigilant_crypto_snatch.marketplace.interface.BuyError'> has occurred: ['EOrder:Insufficient funds']
  • 🟠 An exception of type <class 'vigilant_crypto_snatch.myrequests.HttpRequestError'> has occurred: Connection error in Kraken Ticker
  • 🟡 Could not retrieve a historical price, so cannot determine if strategy “Drop(delay_minutes=10080, drop=15)” was triggered. The original error is: No source could deliver.

These exceptions get caught up in the main loop, and are just reported and then ignored. They have different reporting priorities, but still they are reported in the same fashion. For the user it is rather important to know that there are insufficient funds on the marketplace, but having plain HTTP errors reported is boring. Except when they indicate something that the user could change. So some changes are due in this regard.

Wrapped exceptions

Another self-made problem is that I have wrapped the exceptions. The Kraken marketplace uses the krakenex library, which in turn uses the requests library. It doesn't wrap all the exceptions, so I have to catch these exceptions in my Kraken wrapper:

  • requests.exceptions.ConnectionError

In the Bitstamp wrapper I have to catch these:

  • bitstamp.client.BitstampError
  • requests.exceptions.ChunkedEncodingError
  • requests.exceptions.HTTPError
  • urllib3.exceptions.ProtocolError

In my own wrapper for requests I catch these exceptions:

  • requests.exceptions.ConnectionError
  • requests.exceptions.ReadTimeout
  • requests.exceptions.HTTPError

In the data storage part I have these:

  • sqlalchemy.exc.OperationalError
  • sqlalchemy.orm.exc.NoResultFound

Because my components use each other, they also inherit some of the exceptions. In order to isolate them from each other, I have defined special exceptions in the interfaces of the components:

  • BuyError
  • DatastoreException
  • HistoricalError
  • HttpRequestError
  • TelegramBotException
  • TickerError
  • WithdrawalError

These then wrap the above exceptions into component specific ones. But there are only one or few exception types per component, which means that I need to pool external exceptions. I attach the other exception as context using the raise … from … syntax. This is not really put through to the user, it is only visible in the stack trace. That doesn't do any good within the program though.

Use cases

Perhaps it is best to think this from the end-user perspective. What should that person see as errors? The errors and warnings are ideally actionable. This way I can display specific warnings, and also filter for severity outside of the component.

These are some use cases where an exception could be raised in the marketplace component:

  • The user doesn't have enough funds to make a transaction.
  • Some external server has changed the API and the request doesn't work any more.
  • There is a random internet glitch.
  • There is a service outage.
  • The setup of the marketplace is incorrect, say the wrong API key.

In all of these cases, I likely want to have a different exception. And in order to have them discernible within the program, they need to be distinct exception types.

At the moment, all of these error cases would just raise a BuyError, where I would put the string response from the Kraken marketplace into it, yielding an exception string like ['EOrder:Insufficient funds']. Surely we humans can read that, but it is not as nice as I imagine that it could be. That should rather be an InsufficientFundsError. So let's do that!

Insufficient funds error

My wrapper of the krakenex library for the buy order looks like this:

    def place_order(self, coin: str, fiat: str, volume: float) -> None:
        arguments = {
            "pair": f"{map_normal_to_kraken(coin)}{fiat}",
            "ordertype": "market",
            "type": f"buy",
            "volume": str(volume),
            "oflags": "fcib" if self.prefer_fee_in_base_currency else "fciq",
        }
        try:
            answer = self.handle.query_private("AddOrder", arguments)
        except requests.exceptions.ConnectionError as e:
            raise WithdrawalError("Connection error in Kraken AddOrder") from e
        raise_error(answer, BuyError)

The interesting part is the raise_error function, which creates the exception if needed:

def raise_error(answer: dict, exception: Type[Exception]):
    if len(answer["error"]) > 0:
        raise exception(answer["error"])

This is okay, because it catches all possible errors that are there. Yet it places the burden onto the user.

We can define a new exception type:

class InsufficientFundsError(BuyError):
    pass

And then use that for specific errors:

def raise_error(answer: dict, exception: Type[Exception]) -> None:
    errors = answer["error"]
    if errors:
        if 'EOrder:Insufficient funds' in errors:
            raise InsufficientFundsError()
        else:
            raise exception(answer["error"])

This retains the function signature, it just uses the more specific exception type.

Unfortunately the API documentation for “add order” doesn't state which error strings are possible. Perhaps over time we will find out.

Using this special exception I can pause triggers with insufficient funds for 24 hours, without doing another retry. This makes more sense, because it isn't just a network glitch, but reproducible.

Connection errors

Then there are a lot of pieces of code like this:

        try:
            answer = self.handle.query_private("AddOrder", arguments)
        except requests.exceptions.ConnectionError as e:
            raise WithdrawalError("Connection error in Kraken AddOrder") from e

The krakenex package doesn't wrap the exceptions from the request package, but just lets them get raised through the package boundary into my code. I catch them in my adapter code, such that it doesn't bubble up further. I don't want to import the requests library within my code only to catch the exceptions. Therefore I must wrap them.

Yet I don't have to wrap it into a WithdrawalError. It surely occurs within the withdrawal process, but that is not really that meaningful to the user. The calling code likely knows that it has triggered a withdrawal, so we should rather make sure that the actual problem is transported better.

More telling would be a MarketplaceConnectionError, which we in most cases just want to ignore. But do we want to separate the types of errors in such a way, or should be rather have a ConnectionError as a core entity and use that all over the code? This way of dependency would be fine from an architectural point of view, because it goes inward.

When we are in the main use case, the watch loop, do we care where the connection error came from? The watch loop just checks whether the trigger has cooled off, whether it's triggered and then executes it. No matter what error comes up, it will just report it and go on. Inside the BuyTrigger, we don't really care what has failed. When the historical API cannot deliver, we can't do anything. If there is an issue with the marketplace, we cannot perform the trade. So from that perspective we can just ignore all these errors and move on. But that puts knowledge about the way that the errors are handled in the controller into the adapter code, which doesn't make so much sense either. Yet having tons of specific exceptions will mean that in the watch loop one has to catch all of them. Introducing a common superclass only connects them in the same way of having a common “benign HTTP error” class, which is used everywhere.

I now use my HttpRequestError for all these errors, but attach the underlying error to it. This way the context is not lost for debugging. I log the full exceptions in the debug logging level now, such that I can read them in the system log.