Better error messages

[whyrusleeping]

In a discussion on IRC last night, it was brought up that our error messages kinda suck sometimes. I tend to agree (merkledag: not found).

To improve this, we can distinguish between a few different failure modes for common commands:

1. Cannot find any providers for the given hash
2. Cannot connect to any providers for the given hash
3. MERKLEDAG: NOT FOUND (or maybe: failed to find hash)

Then, if the daemon is offline, we can return something like:
data not found locally.

(for context, i also just read this article and enjoyed it)

[djdv]

Bringing to attention get

>ipfs get invalid
03:49:44.027 ERROR core/comma: received command error in:
goroutine 25 [running]:
runtime/debug.Stack(0x0, 0x0, 0xc042044780)
        C:/Go/src/runtime/debug/stack.go:24 +0xae
github.com/ipfs/go-ipfs/core/commands/e.New(0x14619a0, 0xc042047ae0, 0x0, 0x0, 0x0, 0x0, 0x0)
        C:/tmp/src/github.com/ipfs/go-ipfs/core/commands/e/error.go:29 +0x5d
github.com/ipfs/go-ipfs/core/commands.glob..func61.1(0x146dce0, 0xc04215df10, 0x1470320, 0xc0422aa180, 0x1476ac0, 0xc04214e600)
        C:/tmp/src/github.com/ipfs/go-ipfs/core/commands/get.go:113 +0xb2
created by github.com/ipfs/go-ipfs/core/commands.glob..func61
        C:/tmp/src/github.com/ipfs/go-ipfs/core/commands/get.go:108 +0x1e7
 get.go:113

[kevina]

Fixing the merkeldag error messages was discused is in pr. #4099 which seams to have been abandoned. I will be happy to do what I discussed in that issue and take a pass at giving more informative error messages.

[kevina]

I spend some time working with this, I will create a p.r. in a few days once I figure out something I am reasonable happy with,

[picasso250]

Error: merkledag: not found

centos 7
ipfs version 0.4.13

[picasso250]

After daemon-ed, problem solved.

[hsanjuan]

@Stebalien many things have changed but the merkledag: not found error remains the same.

I would assume that we can now take advantange of errors.Is() and wrap original NotFound errors on higher layers like NotFoundOffline. Do you have an idea of what the best approach would be to address this one?

[Stebalien]

That helps but isn't necessary. We can just fix this.

[hsanjuan]

That helps but isn't necessary. We can just fix this.

It seems that proposed fixes so far have not suceeded to land. How do you want to fix it?

[Stebalien]

The proposed fix modified the core ErrNotFound error to say "we're offline". The correct solution would involve:

• Checking for this error inside the CoreAPI functions themselves, then checking to see if we're offline, then wrapping the error with a hint.
• Possibly changing the merkledag error text to "block not found".
• Wrapping the error inside go-unixfs to indicate that a part of the file was not found.

[hsanjuan]

* Possibly changing the merkledag error text to "block not found".

Let's start here: ipfs/go-ipld-format#55

[lidel]

This came up during triage today.

Inconsistency in errors returned from commands

• the errors we return today are a mix of CLI-specific messages
  • example: gateway https://ipfs.io/ipfs/Qme7TddCpkz1CZLscPhfDQ9VuarjzThiQ42uNucDPmShKv/lol returns CLI-specific ipfs resolve -r /ipfs/Qme7TddCpkz1CZLscPhfDQ9VuarjzThiQ42uNucDPmShKv/lol: no link named "lol" under Qme7TddCpkz1CZLscPhfDQ9VuarjzThiQ42uNucDPmShKv which is pretty bizarre
• ... or try to be overly helpful, but then fail anyway and get constantly changed
  • example: error when mangled CIDv0 in lowercase is detected: path: give a more helpful error when a case-coverted CIDv0 is likely go-path#45

The lack of deterministic codes makes it impossible to reliably consume errors in userland. Folks will match plaintext message but this may break in the future when someone changes the error message.

Proposed suggestion

• @petar noted that we should use well-defined error codes as canonical default, and document their meaning in an outside document (eg /docs/error-codes.md)
  • prior art: https://cloud.google.com/apis/design/errors#handling_errors (feel free to provide more good examples in comments below 🙏)
• CLI tools and gateways could then make a decision to return them as-is, or include at the beginning of error messages
  • eg. {ERROR_CODE}: no link named "lol" under Qme7TddCpkz1CZLscPhfDQ9VuarjzThiQ42uNucDPmShKv

[hsanjuan]

Note that #7074 has the tracking around resolving the Block Not Found errors and the changes would allow each layer to better set the errors as they bubble.

[Winterhuman]

A user in the Matrix room recently encountered this, here's the issue I created before being pointed to this one: #9179

I disagree with "Then, if the daemon is offline, we can return something like:
data not found locally.". In reality, "data not found locally" also doesn't tell the user that they need to start a daemon, it's assuming that they know Kubo needs a daemon which isn't always true, and it's wording makes it seem more like an issue with the software being broken rather than something the user can fix.

Giving an error message which directly states that the daemon isn't running, and it should probably give the ipfs daemon command as well, is going to be the most effective way to inform users in the least ambiguous way.