Improve token-metadata-creator error messages

[sevanspowell]

• As per Testing decimals #26, improve error messages when failing to parse a decimals value.
• For well-known properties, we may be wanted to parse a String or some other type. I've improved the error messages by first trying to parse as an arbitrary JSON value, and it that fails with some "not a valid json value"-style errors, try to parse as a string.
• It's only a quick fix, but it does provide better results:

$ token-metadata-creator entry ... --decimals "potato"
- option --decimals: Error in $: Failed reading: not a valid json value
+ option --decimals: Error in $: parsing Int failed, expected Number, but encountered String

$ token-metadata-creator entry ... --decimals 0x
- option --decimals: Error in $: endOfInput
+ option --decimals: Error in $: parsing Int failed, expected Number, but encountered String


$ token-metadata-creator entry ... --decimals 0.x
- option --decimals: Error in $: Failed reading: takeWhile1
+ option --decimals: Error in $: parsing Int failed, expected Number, but encountered String

[rvl]

Looks like it will work. 👍

[piotr-iohk]

LGTM.

Thanks @sevanspowell!

BTW, do you have any clue why this other npm tests that I mentioned in #26 fails?

  1 failing

  1) token-metadata-creator
       Minimal workflow
         Add required fields:

      AssertionError: expected { Object (sequenceNumber, value, ...) } to deeply equal { Object (sequenceNumber, signatures, ...) }
      + expected - actual

       {
         "sequenceNumber": 0
         "signatures": []
      -  "value": "������"
      +  "value": "ギル"
       }

I think this used to work before 🤔

[rvl]

I suspect it's a genuine failure because of the change to string parsing on the CLI.
Text is always UTF-8 under JSON decoding, but the user's locale might be different.

[sevanspowell]

Hmm, I ran those test locally and they all passed so I assumed they worked. Let me double check for you.

[sevanspowell]

Hmm, I ran those test locally and they all passed so I assumed they worked. Let me double check for you.

Confirmed @piotr-iohk, those npm/nodejs tests complete successfully 👍

[rvl]

Try reproducing it this way:

LANG=C npm run test

[rvl]

Here's an approximate fix for the test failure.

[rvl]

Suggested change

	asJSONString :: String -> Either String p
	asJSONString = Aeson.parseEither parseWellKnown . Aeson.String . T.pack
	asJSONString :: ByteString -> Either String p
	asJSONString = (Aeson.parseEither parseWellKnown . bimap show Aeson.String) <=< T.decodeUtf8'

Use Data.Text.Encoding.decodeUtf8' to ensure that text is always treated as UTF-8.

[rvl]

Suggested change

	asJSONValue :: String -> Either String p
	asJSONValue = (Aeson.parseEither parseWellKnown =<<) . Aeson.eitherDecodeStrict' . BC8.pack
	asJSONValue :: ByteString -> Either String p
	asJSONValue = Aeson.parseEither parseWellKnown <=< Aeson.eitherDecodeStrict'

[sevanspowell]

Hmm, that solution just moves the decoding error to the UTF-8 locale. The C locale works, but UTF-8 throws the same error. I'll look into the character encoding a bit more, I imagine I'm going to have to normalize the input stream.

[sevanspowell]

Which is exactly what you were saying... Sorry.

[rvl]

No prob!

[rvl]

I added the utf-8 locale fix that we have in cardano-wallet.
The problem with testing is that LANG=C also affects nodejs, so 🤷.
Let's merge.

[piotr-iohk]

@rvl @sevanspowell this is something I'm also able to reproduce manually. My default bash locale is:

$ echo $LANG
en_US.UTF-8

(so, I guess, nothing fancy)

Doing:

$ token-metadata-creator entry --init 919e8a1922aaa764b1d66407c6f62244e77081215f385b60a62091494861707079436f696e

$ token-metadata-creator entry 919e8a1922aaa764b1d66407c6f62244e77081215f385b60a62091494861707079436f696e \
  --name "ギル" \
  --description "Locale test!!!" \
  --policy /home/piotr/wb/tokens_minter/testnet/happy_coin/policy.script

results in this in the *.json.draft:

...
    "name": {
        "sequenceNumber": 0,
        "value": "������",
        "signatures": []
    }
...

[sevanspowell]

@piotr-iohk is that with the changes @rvl committed a few minutes ago (61a4ab8)? I've just tested it myself with my bash locale the same as yours, but it worked for me.

[rvl]

Ah yes @piotr-iohk - good catch - I found there was something else missing in the locale-setting code.
Now it works, and I added a test case for it.

[piotr-iohk]

@sevanspowell @rvl yes I was checking just before latest commits.
On the latest version 61a4ab8 it works fine 🎉.