frostfs-adm: Add info to error messages #1386

Merged
dstepanov-yadro merged 1 commit from potyarkin/frostfs-node:feature/better-errors into master 2024-09-23 07:47:04 +00:00
Member

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
All checks were successful
Tests and linters / Run gofumpt (pull_request) Successful in 50s
DCO action / DCO (pull_request) Successful in 1m0s
Pre-commit hooks / Pre-commit (pull_request) Successful in 1m54s
Vulncheck / Vulncheck (pull_request) Successful in 1m52s
Build / Build Components (pull_request) Successful in 2m15s
Tests and linters / gopls check (pull_request) Successful in 2m23s
Tests and linters / Staticcheck (pull_request) Successful in 2m47s
Tests and linters / Tests (pull_request) Successful in 4m0s
Tests and linters / Tests with -race (pull_request) Successful in 5m18s
Tests and linters / Lint (pull_request) Successful in 2m39s
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
Dismissed
@ -30,3 +32,3 @@
if err := c.SendCommitteeTx(w.Bytes(), false); err != nil {
return err
return fmt.Errorf("committee transaction: %w", err)

send committee transaction - to distinguish between errors

`send committee transaction` - to distinguish between errors
potyarkin marked this conversation as resolved
@ -35,1 +37,3 @@
return c.AwaitTx()
err := c.AwaitTx()
if err != nil {
err = fmt.Errorf("committee transaction: %w", err)

await committee transaction - to distinguish between errors

`await committee transaction` - to distinguish between errors
potyarkin marked this conversation as resolved
potyarkin force-pushed feature/better-errors from 26b65a0141 to d79537edbb 2024-09-20 15:58:39 +00:00 Compare
potyarkin requested review from dstepanov-yadro 2024-09-20 16:04:50 +00:00
realloc requested review from aarifullin 2024-09-23 06:02:06 +00:00
acid-ant requested review from storage-core-committers 2024-09-23 06:29:22 +00:00
acid-ant requested review from storage-core-developers 2024-09-23 06:29:22 +00:00
dstepanov-yadro approved these changes 2024-09-23 06:35:37 +00:00
acid-ant approved these changes 2024-09-23 06:36:13 +00:00
achuprov approved these changes 2024-09-23 06:39:54 +00:00
aarifullin approved these changes 2024-09-23 07:44:37 +00:00
dstepanov-yadro merged commit f71418b73c into master 2024-09-23 07:47:04 +00:00
potyarkin deleted branch feature/better-errors 2024-09-23 08:44:53 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
5 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-node#1386
No description provided.