fix(gnovm): show "out of gas" errors (#2365)

[grepsuzette]

The following should fix #2365 when adding a package.

Problem

2365 (and before it, #1205) described a way where adding packages with insufficient gas very often lead to hard to read errors such as:

Data: internal error
Msg traces:
    0 /me/gno/tm2/pkg/crypto/keys/client/maketx.go:213 - deliver transaction failed: log:recovered: strings/strings.gno:11: {WritePerByte}

Diagnostic

Using the call stack in #2365, the following seems to happen.

While preprocessing some file, a panic with OutOfGasException occurs: https://github.com/gnolang/gno/blob/e7e47d2589ceece37b0502d0df8d54ded582d501/tm2/pkg/store/types/gas.go#L96-L98

Upper level recovers and repanics with error: https://github.com/gnolang/gno/blob/e7e47d2589ceece37b0502d0df8d54ded582d501/gnovm/pkg/gnolang/preprocess.go#L3057-L3070

Upper level treats it as a banal error (falling in default: rather than OutOfGasException, this snippet is longer, make sure to scroll down): https://github.com/gnolang/gno/blob/e7e47d2589ceece37b0502d0df8d54ded582d501/tm2/pkg/sdk/baseapp.go#L733-L752

Proposed fix

Just add a couple of lines in gno/gnovm/pkg/gnolang/preprocess.go:

func predefineNow(store Store, last BlockNode, d Decl) (Decl, bool) {
    defer func() {
        if r := recover(); r != nil {
            // before re-throwing the error, append location information to message.
            loc := last.GetLocation()
            if nline := d.GetLine(); nline > 0 {
                loc.Line = nline
            }
            if rerr, ok := r.(error); ok {
                // NOTE: gotuna/gorilla expects error exceptions.
                panic(errors.Wrap(rerr, loc.String()))
+           } else if ex, ok := r.(OutOfGasException); ok {
+                panic(ex)
            } else {
                // NOTE: gotuna/gorilla expects error exceptions.
                panic(fmt.Errorf("%s: %v", loc.String(), r))
            }
        }
    }()
    m := make(map[Name]struct{})
    return predefineNow2(store, last, d, m)
}

After recompiling and testing with the modus operandi described in #2365, the error message is now:

Data: out of gas error
Msg Traces:
    0  /me/gno/tm2/pkg/crypto/keys/client/maket
x.go:213 - deliver transaction failed: log:out of gas, gasWanted: 9000000, gasUsed: 9001550 location: WritePerByte

Repeatedly calling with fresh gnodev -minimal each time and adding package now consistently shows this message.

Discussion

There still remains a question about the "NOTE: gotuna/gorilla expects error exceptions".

Think it has to do with gnoweb ability to print failures, e.g. gnoweb uses func makeRequest(log *slog.Logger, cfg *Config, qpath string, data []byte) (res *abci.ResponseQuery, err error).

However BaseApp.runTx will turn the OutOfGasException to a nice ABCIError(std.ErrOutOfGas()). So, in theory, it should do.

I will do some more test tonight adding many packages from /p/demo.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Project coverage is 54.63%. Comparing base (e7e47d2) to head (edc0f3b).

Files	Patch %	Lines
gnovm/pkg/gnolang/preprocess.go	0.00%	1 Missing and 1 partial :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #2368 +/- ## ========================================== - Coverage 54.63% 54.63% -0.01% ========================================== Files 581 581 Lines 77967 77969 +2 ========================================== Hits 42598 42598 - Misses 32190 32191 +1 - Partials 3179 3180 +1 ``` | [Flag](https://app.codecov.io/gh/gnolang/gno/pull/2368/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gnolang) | Coverage Δ | | |---|---|---| | [gnovm](https://app.codecov.io/gh/gnolang/gno/pull/2368/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gnolang) | `60.05% <0.00%> (-0.01%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=gnolang#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[ltzmaxwell]

just curious, is it only happening with predefineNow()?

[grepsuzette]

just curious, is it only happening with predefineNow()?

I am not sure, that's why I will try to add many packages.