Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(gnokey): print verbose errors in CLI #806

Merged
merged 11 commits into from
May 15, 2023
Merged

Conversation

moul
Copy link
Member

@moul moul commented May 7, 2023

Wrapped errors (particularly abci ones) were displayed partially.

This PR extends tm2/pkg/errors to enhance error information when using the %+v error format. It offers detailed message traces similar to %#v, but in a more user-friendly manner. Furthermore, it includes stack traces for abciError from CLI in the output.

Before

$ go run ./gno.land/cmd/gnokey maketx addpkg --pkgpath gno.land/p/demo/audio/biquad --pkgdir examples/gno.land/p/demo/audio/biquad --deposit 100000000ugnot --gas-fee 1000000ugnot --gas-wanted 2000000 --broadcast --chainid dev --remote localhost:26657 moula
Enter password.
internal error

After

$ go run ./gno.land/cmd/gnokey maketx addpkg --pkgpath gno.land/p/demo/audio/biquad --pkgdir examples/gno.land/p/demo/audio/biquad --deposit 100000000ugnot --gas-fee 1000000ugnot --gas-wanted 2000000 --broadcast --chainid dev --remote localhost:26657 moula
Enter password.
--= Error =--
Data: internal error
Msg Traces:
    0  /home/moul/go/src/github.com/gnolang/gno/tm2/pkg/crypto/keys/client/addpkg.go:211 - deliver transaction failed: log:recovered: package already exists: gno.land/p/demo/audio/biquad
stack:
goroutine 44 [running]:
runtime/debug.Stack()
        /nix/store/ndlw8ji6g2s0vqn7fivkb8sz7zfd67bm-go-1.20.3/share/go/src/runtime/debug/stack.go:24 +0x65
github.com/gnolang/gno/tm2/pkg/sdk.(*BaseApp).runTx.func1()
        /home/moul/go/src/github.com/gnolang/gno/tm2/pkg/sdk/baseapp.go:746 +0x3aa
panic({0xc22a60, 0xc0008c2900})
        /nix/store/ndlw8ji6g2s0vqn7fivkb8sz7zfd67bm-go-1.20.3/share/go/src/runtime/panic.go:884 +0x213
github.com/gnolang/gno/tm2/pkg/sdk/vm.(*VMKeeper).AddPackage(_, {{0xf95ca8, 0xc006a763f0}, 0x2, {0xf958a0, 0xc00520db50}, {0xf95e30, 0xc008eaeb00}, {0xc0060e2fbd, 0x3}, ...}, ...)
        /home/moul/go/src/github.com/gnolang/gno/tm2/pkg/sdk/vm/keeper.go:141 +0xadd
github.com/gnolang/gno/tm2/pkg/sdk/vm.vmHandler.handleMsgAddPackage({_}, {{0xf95ca8, 0xc006a763f0}, 0x2, {0xf958a0, 0xc00520db50}, {0xf95e30, 0xc008eaeb00}, {0xc0060e2fbd, 0x3}, ...}, ...)
        /home/moul/go/src/github.com/gnolang/gno/tm2/pkg/sdk/vm/handler.go:46 +0x2a5
github.com/gnolang/gno/tm2/pkg/sdk/vm.vmHandler.Process({_}, {{0xf95ca8, 0xc006a763f0}, 0x2, {0xf958a0, 0xc00520db50}, {0xf95e30, 0xc008eaeb00}, {0xc0060e2fbd, 0x3}, ...}, ...)
        /home/moul/go/src/github.com/gnolang/gno/tm2/pkg/sdk/vm/handler.go:27 +0x205
github.com/gnolang/gno/tm2/pkg/sdk.(*BaseApp).runMsgs(_, {{0xf95ca8, 0xc006a763f0}, 0x2, {0xf958a0, 0xc00520db50}, {0xf95e30, 0xc008eaeb00}, {0xc0060e2fbd, 0x3}, ...}, ...)
        /home/moul/go/src/github.com/gnolang/gno/tm2/pkg/sdk/baseapp.go:644 +0x2ed
github.com/gnolang/gno/tm2/pkg/sdk.(*BaseApp).runTx(0xc000196400, 0x2, {0xc002a2b180, 0x52f, _}, {{0xc00520cfc0, 0x1, 0x1}, {0x1e8480, {{0xc003962cb7, ...}, ...}}, ...})
        /home/moul/go/src/github.com/gnolang/gno/tm2/pkg/sdk/baseapp.go:826 +0x965
github.com/gnolang/gno/tm2/pkg/sdk.(*BaseApp).DeliverTx(0x0?, {{}, {0xc002a2b180?, 0x0?, 0x0?}})
        /home/moul/go/src/github.com/gnolang/gno/tm2/pkg/sdk/baseapp.go:580 +0x17d
github.com/gnolang/gno/tm2/pkg/bft/abci/client.(*localClient).DeliverTxAsync(0xc006b74840, {{}, {0xc002a2b180, 0x52f, 0x52f}})
        /home/moul/go/src/github.com/gnolang/gno/tm2/pkg/bft/abci/client/local_client.go:82 +0xfc
github.com/gnolang/gno/tm2/pkg/bft/proxy.(*appConnConsensus).DeliverTxAsync(0x0?, {{}, {0xc002a2b180?, 0x0?, 0x0?}})
        /home/moul/go/src/github.com/gnolang/gno/tm2/pkg/bft/proxy/app_conn.go:73 +0x26
github.com/gnolang/gno/tm2/pkg/bft/state.execBlockOnProxyApp({0xf96b60, 0xc008a6f620}, {0xf99210, 0xc008303010}, 0xc00629d6c0, {0xf9ead0, 0xc000b9e1f8})
        /home/moul/go/src/github.com/gnolang/gno/tm2/pkg/bft/state/execution.go:253 +0x5aa
github.com/gnolang/gno/tm2/pkg/bft/state.(*BlockExecutor).ApplyBlock(_, {{0xd3785c, 0xb}, {0xd3785c, 0xb}, {0x0, 0x0}, {0xc008cd9b6b, 0x3}, 0x199f, ...}, ...)
        /home/moul/go/src/github.com/gnolang/gno/tm2/pkg/bft/state/execution.go:102 +0x115
github.com/gnolang/gno/tm2/pkg/bft/consensus.(*ConsensusState).finalizeCommit(0xc00853c600, 0x19a0)
        /home/moul/go/src/github.com/gnolang/gno/tm2/pkg/bft/consensus/state.go:1347 +0x97e
github.com/gnolang/gno/tm2/pkg/bft/consensus.(*ConsensusState).tryFinalizeCommit(0xc00853c600, 0x19a0)
        /home/moul/go/src/github.com/gnolang/gno/tm2/pkg/bft/consensus/state.go:1275 +0x313
github.com/gnolang/gno/tm2/pkg/bft/consensus.(*ConsensusState).enterCommit.func1()
        /home/moul/go/src/github.com/gnolang/gno/tm2/pkg/bft/consensus/state.go:1221 +0x97
github.com/gnolang/gno/tm2/pkg/bft/consensus.(*ConsensusState).enterCommit(0xc00853c600, 0x19a0, 0x0)
        /home/moul/go/src/github.com/gnolang/gno/tm2/pkg/bft/consensus/state.go:1252 +0xb03
github.com/gnolang/gno/tm2/pkg/bft/consensus.(*ConsensusState).addVote(0xc00853c600, 0xc00957ee60, {0x0, 0x0})
        /home/moul/go/src/github.com/gnolang/gno/tm2/pkg/bft/consensus/state.go:1637 +0x913
github.com/gnolang/gno/tm2/pkg/bft/consensus.(*ConsensusState).tryAddVote(0xc00853c600, 0xf?, {0x0?, 0x0?})
        /home/moul/go/src/github.com/gnolang/gno/tm2/pkg/bft/consensus/state.go:1483 +0x27
github.com/gnolang/gno/tm2/pkg/bft/consensus.(*ConsensusState).handleMsg(0xc00853c600, {{0xf8f460, 0xc001efa870}, {0x0, 0x0}})
        /home/moul/go/src/github.com/gnolang/gno/tm2/pkg/bft/consensus/state.go:691 +0x318
github.com/gnolang/gno/tm2/pkg/bft/consensus.(*ConsensusState).receiveRoutine(0xc00853c600, 0x0)
        /home/moul/go/src/github.com/gnolang/gno/tm2/pkg/bft/consensus/state.go:650 +0x42c
created by github.com/gnolang/gno/tm2/pkg/bft/consensus.(*ConsensusState).OnStart
        /home/moul/go/src/github.com/gnolang/gno/tm2/pkg/bft/consensus/state.go:344 +0x4af
--= /Error =--

Maybe we should print this format selectively, exclusively for abciError.

@moul moul self-assigned this May 7, 2023
@moul moul marked this pull request as ready for review May 7, 2023 21:07
@moul moul requested a review from a team as a code owner May 7, 2023 21:07
@moul moul changed the title chore(gnokey): return print verbose errors in CLI chore(gnokey): print verbose errors in CLI May 7, 2023
@thehowl
Copy link
Member

thehowl commented May 8, 2023

Hmm. What if instead we changed the behaviour of tm2/pkg/errors/errors.go, (*cmnError).Format to show the full error with data and stacktrace also for the %+v verb?

When thinking about errors in general, I don't think we should display them as %#v, as for many errors it's bound to have less useful or clutter information when compared to %v/%+v.

Signed-off-by: Manfred Touron <94029+moul@users.noreply.github.com>
@moul-bot moul-bot force-pushed the dev/moul/verbose-cli-errors branch from 9445eee to 96762c2 Compare May 10, 2023 16:17
@moul moul marked this pull request as draft May 12, 2023 07:39
moul added 5 commits May 12, 2023 09:40
Signed-off-by: Manfred Touron <94029+moul@users.noreply.github.com>
Signed-off-by: Manfred Touron <94029+moul@users.noreply.github.com>
Signed-off-by: Manfred Touron <94029+moul@users.noreply.github.com>
Signed-off-by: Manfred Touron <94029+moul@users.noreply.github.com>
@moul moul marked this pull request as ready for review May 12, 2023 07:56
@moul moul requested a review from jaekwon as a code owner May 12, 2023 07:56
tm2/pkg/errors/errors.go Outdated Show resolved Hide resolved
tm2/pkg/errors/errors.go Outdated Show resolved Hide resolved
tm2/pkg/errors/errors.go Show resolved Hide resolved
moul and others added 5 commits May 12, 2023 17:11
Signed-off-by: Manfred Touron <94029+moul@users.noreply.github.com>
Co-authored-by: Morgan <morgan@morganbaz.com>
Co-authored-by: Morgan <morgan@morganbaz.com>
Signed-off-by: Manfred Touron <94029+moul@users.noreply.github.com>
Copy link
Member

@zivkovicmilos zivkovicmilos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 💯

I know a lot of people have been struggling with vague internal error messages - I guess not anymore 🚀

@moul moul merged commit ada801c into master May 15, 2023
@moul moul deleted the dev/moul/verbose-cli-errors branch May 15, 2023 13:58
moul-bot pushed a commit that referenced this pull request May 19, 2023
Co-authored-by: Morgan <morgan@morganbaz.com>
@moul moul added this to the 🌟 main.gno.land (wanted) milestone Sep 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Archived in project
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants