fix: improved error message on broken CIDv0 #33
Conversation
path.go
Outdated
| func decodeCid(cstr string) (cid.Cid, error) { | ||
| c, err := cid.Decode(cstr) | ||
| if err != nil && len(cstr) == 46 && cstr[:2] == "qm" { // https://github.com/ipfs/go-ipfs/issues/7792 | ||
| return cid.Cid{}, fmt.Errorf("%v: This looks like a CIDv0 that has been lowercased. Convert the CIDv0 to CIDv1 in case-insensitive base32 using 'ipfs cid base32 <Qm..>' and try again.", err) |
There was a problem hiding this comment.
I would still like this to be much shorter. Can we just say "this looks like a lowercase CID"? Telling the user what to do is a bit out of scope for an error.
There was a problem hiding this comment.
'This looks like a CIDv0 that has been lowercased; you may wish to convert to case-agnostic CIDv1' or similar? Intent would be not to tell user what to do, but hint that there's a workaround if preserving case isn't an option.
|
That works. Could also say "consider converting it to base32 first". We
can also include short links.
|
|
pushed a fixup with:
How's that sound? |
|
It should start lower case and I'd prefer if it were a bit shorter, but that's not a huge issue. |
|
how's this for a compromise between size and communicating what's wrong @Stebalien @jessicaschilling? The construction does get a bit whacky when you look at it in full, which is why I'm trying out the parens: invalid path "/ipfs/qmbwqxbekc3p8tqskc98xmwnzrzdtrlmimpl8wbutgsmnr": invalid CID: selected encoding not supported (possible lowercased CIDv0; consider converting to a case-agnostic CIDv1, such as base32) |
|
SGTM - it's a little awkward but good enough given the constraints. |
|
LGTM! (although the test needs fixing). I generally avoid long error messages as we tend to glue them together. |
rvagg
force-pushed
the
rvagg/bad-cidv0
branch
from
bfc027d to
5e8ad22
Compare
|
Sorry, I hadn't properly pushed the version I had locally which had it with parens and the test working. I've squashed now and force pushed the single commit, the message is I'll take this as approved and merge shortly. |
|
too slow 😛 |
|
@Stebalien @aschmahmann would it be ok to make a |
|
@lidel SGTM |
fix: improved error message on broken CIDv0 This commit was moved from ipfs/go-path@3e8c1a8
move @lidel's fix from ipfs/go-cid#116