ARC-65: AVM Run Time Errors based on program bytecode

[cusma]

This ARC formalizes the conventions to rise AVM run time errors based on program bytecode.

[joe-p]

I think it would make sense to include a special prefix to the log to signify the fact that it is an error message. This would remove the requirement of parsing the error message for the last three TEAL lines to determine if the last log was indeed an error message, which seems a bit fragile to me.

On a related note, I think we should include a reference implementation for the showing the error message client-side.

Otherwise I don't have a problem with the ARC but it's not really clear to me where this would be used. In order to interact with an ARC4 method you need the signature, and if you can share the signature you might as well share ARC56 which has the source mapping with errors. I think it would be good to expand upon the motivation section with scenarios in which this is preferable.

[joe-p]

Scratch that last part. I could imagine this being useful where you have an app ID for an ARC implementation where the implementation varies so you might not necessarily have the source map. Ie. ARC200 tokens.

Still, I think it would be useful to add some more context to the ARC describing these sort of scenarios.

[robdmoore]

From memory you only get logs back from a failed transaction if you do a simulate call (been a while since I checked so I could be wrong)? AlgoKit utils does have some logic to perform simulate on error (if debug config enabled). Also, the nice thing about this is the current pattern which is to use a TEAL comment and then if source maps are present so the 3 lines surrounding the error line would work nicely still with this model (and in fact would also work if you did a disassembly too?)

[robdmoore]

Long story short: I like this technique

[cusma]

I think it would make sense to include a special prefix to the log to signify the fact that it is an error message. This would remove the requirement of parsing the error message for the last three TEAL lines to determine if the last log was indeed an error message, which seems a bit fragile to me.

In the example we suggest the usage of fixed-length error codes (ERR:<code>) or a their "full version" which includes the error message itself (ERR:<code> - <message>).

I think we could formalize this better in the ARC, defining a precise regex for the error code and messages. @nullun WDYT?

@joe-p @nullun do you have suggestion for the error code/message regex?

On a related note, I think we should include a reference implementation for the showing the error message client-side.

It's hard to come up with a general reference implementation, since the program could be anything and contain any error. It was easier to demonstrate a specific example. What do you suggest as reference implementation on top of the existing example?

Otherwise I don't have a problem with the ARC but it's not really clear to me where this would be used. In order to interact with an ARC4 method you need the signature, and if you can share the signature you might as well share ARC56 which has the source mapping with errors. I think it would be good to expand upon the motivation section with scenarios in which this is preferable.

This approach to AVM run time errors simplifies a lot the overhead of generating, synching, shipping and parsing (client side) the Source Map jointly with the Algod API response. Also, this would work also for non-ARC4 Smart Contracts. It's a general approach, closer to the pure AVM (due to the lack of a native error raising opcode).

[nullun]

From memory you only get logs back from a failed transaction if you do a simulate call

This was actually changed a few months back in algorand/go-algorand#5875 and released in v3.22.0, allowing failed transactions to return PC, appID, stack, scratch, and logs. Although goal and I'm pretty sure the SDKs haven't been updated to read or expose this extra data to the user when it does fail. Submitting a transaction to the /v2/transactions API endpoint will return the data shown in this ARC though.

[nullun]

I think it would make sense to include a special prefix to the log to signify the fact that it is an error message. This would remove the requirement of parsing the error message for the last three TEAL lines to determine if the last log was indeed an error message, which seems a bit fragile to me.

In the example we suggest the usage of fixed-length error codes (ERR:<code>) or a their "full version" which includes the error message itself (ERR:<code> - <message>).

I think we could formalize this better in the ARC, defining a precise regex for the error code and messages. @nullun WDYT?

Definitely agree with Joe, this should likely be defined as part of the standard so there's a sane way to parse them. I do like what is already there. ERR:<code> - <message> is straight forward enough and I feel like anyone (with or without a technical background) would understand this is an error message hopefully giving them a reasonable amount of information. If we wanted something more application specific (imagining people might be searching these errors online) maybe the appID could be included somewhere?

@joe-p @nullun do you have suggestion for the error code/message regex?

Something like s/^ERR:(\w*)\s-\s(.*)$/ ?

[joe-p]

ERR: prefix makes sense to me.

One other thing to think about that isn't currently in the ARC is encoding. It's implied that error messages are UTF-8 strings but it should be explicit about that. Or we could think of a way to do UTF-8 strings now but also support event signatures, so this could be combined with something like ARC56 for dynamic error messages that don't take up too many bytes on-chain.

[cusma]

@nullun @joe-p @robdmoore I've updated the ARC based on discussion feedback! If it's ok I'll try to push it for a final call.

[nullun]

I realise this regex wouldn't satisfy the below example of ERR:042 where there isn't a short message.

Maybe the following would be better? Welcome others input here.
s/^ERR:(\w*)(\s?-\s?(.*))?$/

[nullun]

Just realised my example above created a third capturing group. This may be better: ERR:(\w*)(?:\s?-\s?(.*))? Which then gives you 2 capturing groups, the first for the error code, the second for the error message.

[cusma]

Good point! Also: both numeric (ERR:0042) and alphanumeric error codes should be supported (either "labels" or things like "hex" codes, like ERR:BADREQUEST or ERR:0xAA). I'll update the spec!

[nullun]

As discussed, I think changing it to rely on the logs array would be better. Just decode each value and compare against the regex.
Here's an example I did via the command line with an application that attempts to use ABI methods first, then falls back to non-ABI methods. It resulted in two logged errors before failing.

curl -s -X POST http://127.0.0.1:4001/v2/transactions --data-binary @appl_call.stxn | jq -r '."data"."eval-states"[0]."logs"[] | @base64d' | grep -E "ERR:(\w*)(?:\s?-\s?(.*))?"
ERR:01 - Invalid Method
ERR:02 - No arguments supplied