Error handling support in Cadence scripts

[aishairzay]

Issue To Be Solved

This issue is similar to the following issue which mentions the need for a try/catch functionality in Cadence: https://github.com/onflow/cadence/issues/1874

I understand that allowing a transaction to swallow errors makes it very difficult to understand whether or not a transaction should be reverted or not, which makes error handling very difficult to support in a transaction context. But, in a read-only script, I believe we can ignore the possible shortcomings of supporting a try/catch sort of mechanism, and allow for catching errors which will allow for more robust Cadence scripts to be written.

We are running into a problem in writing cadence scripts right now where we are running into errors due to invalid or incomplete resources being in user's accounts.

For example, on the NFT Catalog (https://flow-nft-catalog.com), when using the catalog's list of NFTs to view NFTs owned by an account, we use a Cadence script that loops through all relevant storage paths on a user's account. If one of the storage paths is contained by a contract that is invalid (pre-secure cadence update), the entire script is forced to panic, and is unable to continue running, and we're unable to retrieve NFTs for that account. This is also a problem when an individual contract's implementation panics either on purpose or not on purpose. I.E. implementing an NFT getIDs() to just run panic.

I believe this will become more frequent when storage/public/private path iteration is available as well, as it will be difficult to handle all possible errors in a script that may come up when trying to iterate through an account.

Suggested Solution

I'm not sure what a Cadence version of catching errors would look like. We could simply go with a try { /* code */ } catch (e: (Error|String?)) { /* error here */ } , but i'll leave this choice to those who know better than myself.

[bluesign]

Actually in script context it makes sense. But still try/catch is ugly, maybe something like:

DontPanic<T:Any>((():T)):(():T?)

in big friendly letters.

So calling getIDs() safely will be:

var getIDsSafe = DontPanic(getIDs)
var ids = getIDsSafe() // ids will have optional type: `[UInt64]?`

PS: Not sure if we can handle function parameters there, but worst case we can wrap them in func():T

[bjartek]

Please prioritize this. It causes so many issues when depending on other old contracts while the language is still evolving. If you happen to have sometime created something from ages ago storage iteration brakes. Like on my account.

[SupunS]

Agree that this is a useful feature to have.

I also tend to agree with @bluesign here that try-catch block is kind of becoming an anti-pattern now because people tend to misuse it quite a lot. e.g: wrap unnecessary code blocks with try-catch, other than the actual line they need. So allowing to handle panics at the expression level sounds like a better approach.

I like how Swift does it, using the try-expression (https://docs.swift.org/swift-book/LanguageGuide/ErrorHandling.html)

var ids: T? = try getIDs()

[bluesign]

@SupunS I am also not sure if Cadence can rollback function invocation; it would be tricky for sure. ( if allowed in transactions)

[SupunS]

yeah. I don't know enough about the protocol side of things to really say anything, but yeah, it will be tricky for transactions.

[aishairzay]

I agree this wouldn't be very feasible for transactions without a large amount of effort, but this would be super useful in scripts regardless of if it works in transactions. Is there a way we can have some naive approach to this that works in scripts even if it does not roll back function invocations?

A DontPanic method like @bluesign mentioned would be perfect imo

[austinkline]

Thanks @SupunS for mentioning this here. I am in favor of:

1. Only permit this in scripts. We already do this for methods like getAuthAccount, for instance
2. A helper that makes a return value become optional makes a lot of sense to me.

In the case of iteration, we'd probably need to get three new methods that encapsulate this for iteration since its return type doesn't really fit well with this

fun forEachPublicSafe(_ function: ((PublicPath, Type?): Bool))
fun forEachPrivateSafe(_ function: ((PrivatePath, Type?): Bool))
fun forEachStoredSafe(_ function: ((StoragePath, Type?): Bool))

This has the added benefit of being able to detect paths that are invalid (the type is nil)