frostfs-adm: Add info to error messages #1386

potyarkin · 2024-09-20T10:46:29Z

potyarkin commented

2024-09-20 10:46:29 +00:00

These error messages bubble up to human users - adding more context will help to find the cause of the issue faster.

morph init could error out due to insufficient funds in committee wallet.

Previously this would be shown as:

Error: Insufficient funds (-511) - insufficient funds
Error: script failed (FAULT state) due to an error: at instruction 94 (ASSERT): ASSERT failed

With suggested changes:

Error: committee transaction: Insufficient funds (-511) - insufficient funds
Error: transfer failed: from=committee(NZFj94ednrga3uHBrXCJL77GdAQVA3wdMS) amount=5000000000000: script failed (FAULT state) due to an error: at instruction 94 (ASSERT): ASSERT failed

New error messages are not perfect but they are better that what we have now. Possible improvements:

Show transaction fee in insufficient funds error. I have not found a way to surface that. Fees are a visible deep inside neo-go but not at frostfs-adm level.
Show token names in addition to amounts. We can access Uint160 token ids but I don't know if we have an easy way to translate these ids into human-readable strings.
Show recepients' addresses in the second error message. I didn't add that because recepients are represented by Uint160 hash and for the sender we have full 25-byte base58 encoded address. Using different address formats in a single message would be misleading.

These error messages bubble up to human users - adding more context will help to find the cause of the issue faster. `morph init` could error out due to insufficient funds in committee wallet. Previously this would be shown as: - `Error: Insufficient funds (-511) - insufficient funds` - `Error: script failed (FAULT state) due to an error: at instruction 94 (ASSERT): ASSERT failed` With suggested changes: - `Error: committee transaction: Insufficient funds (-511) - insufficient funds` - `Error: transfer failed: from=committee(NZFj94ednrga3uHBrXCJL77GdAQVA3wdMS) amount=5000000000000: script failed (FAULT state) due to an error: at instruction 94 (ASSERT): ASSERT failed` New error messages are not perfect but they are better that what we have now. Possible improvements: - Show transaction fee in `insufficient funds` error. I have not found a way to surface that. Fees are a visible [deep inside neo-go](https://github.com/nspcc-dev/neo-go/blob/e1d5ac8557d7e087158e6a766d059913a56df79f/pkg/core/mempool/mem_pool.go#L187) but not at frostfs-adm level. - Show token names in addition to amounts. We can access `Uint160` token ids but I don't know if we have an easy way to translate these ids into human-readable strings. - Show recepients' addresses in the second error message. I didn't add that because recepients are represented by `Uint160` hash and for the sender we have full 25-byte base58 encoded address. Using different address formats in a single message would be misleading.

potyarkin added 1 commit 2024-09-20 10:46:30 +00:00

[#1386 ] frostfs-adm: Add info to error messages

Tests and linters / Run gofumpt (pull_request) Successful in 50s

Details

DCO action / DCO (pull_request) Successful in 1m0s

Details

Pre-commit hooks / Pre-commit (pull_request) Successful in 1m54s

Details

Vulncheck / Vulncheck (pull_request) Successful in 1m52s

Details

Build / Build Components (pull_request) Successful in 2m15s

Details

Tests and linters / gopls check (pull_request) Successful in 2m23s

Details

Tests and linters / Staticcheck (pull_request) Successful in 2m47s

Details

Tests and linters / Tests (pull_request) Successful in 4m0s

Details

Tests and linters / Tests with -race (pull_request) Successful in 5m18s

Details

Tests and linters / Lint (pull_request) Successful in 2m39s

Details

26b65a0141

These error messages bubble up to human users - adding more context helps
to find the cause of the issue faster.

Signed-off-by: Vitaliy Potyarkin <v.potyarkin@yadro.com>

potyarkin added the

enhancement

frostfs-adm

P3

labels 2024-09-20 10:54:26 +00:00

dstepanov-yadro requested changes 2024-09-20 13:10:38 +00:00

cmd/frostfs-adm/internal/modules/morph/initialize/initialize_roles.go Outdated

					
				@ -30,3 +32,3 @@

					if err := c.SendCommitteeTx(w.Bytes(), false); err != nil {

						return err

						return fmt.Errorf("committee transaction: %w", err)

dstepanov-yadro commented

2024-09-20 13:07:02 +00:00

send committee transaction - to distinguish between errors

`send committee transaction` - to distinguish between errors

potyarkin marked this conversation as resolved

cmd/frostfs-adm/internal/modules/morph/initialize/initialize_roles.go Outdated

					
				@ -35,1 +37,3 @@

					return c.AwaitTx()

					err := c.AwaitTx()

					if err != nil {

						err = fmt.Errorf("committee transaction: %w", err)