supertokens-core icon indicating copy to clipboard operation
supertokens-core copied to clipboard

Change how Transaction exceptions are handled

Open rishabhpoddar opened this issue 3 years ago • 0 comments

Right now, we throw a StorageTransactionLogicException from the transaction block. This exception wraps a generic Exception type. The problem with this is that if there are specific exceptions that we need to unwrap and throw, those may get forgotten and we may just end up throwing a StorageTransactionLogicException -> yielding a 500 error in the API (even though it should not be that for that specific exception type).

To solve this, we can create a rule that a function should never throw a StorageTransactionLogicException, but then that means it would have to throw Exception since when we unwrap StorageTransactionLogicException, we get Exception. This will make things worse.

Therefore I propose the following:

  • We make StorageTransactionLogicException abstract and create two (non-abstract) children - GenericStorageTransactionLogicException and SpecificStorageTransactionLogicException.
  • In the transaction itself, we wrap all "expected" exceptions in SpecificStorageTransactionLogicException and all other exceptions in GenericStorageTransactionLogicException.
  • We enforce (via linter or something else) that a function never throws StorageTransactionLogicException or SpecificStorageTransactionLogicException. But only GenericStorageTransactionLogicException (which will always yield a 500 error from the API).
  • We will be forced to try catch all transactions with StorageTransactionLogicException, and then check the instance type of it to be SpecificStorageTransactionLogicException or GenericStorageTransactionLogicException. If it's the Specfic... one, then we can unwrap it and throw the underlying exception, but if it's the Generic..., then we just throw that.

rishabhpoddar avatar Aug 27 '21 06:08 rishabhpoddar