paritytech / capi

[WIP] A framework for crafting interactions with Substrate chains
https://docs.capi.dev
Apache License 2.0
105 stars 10 forks source link

safeguard against invalid string supplied to `chain.blockHash` #1127

Open harrysolovay opened 1 year ago

harrysolovay commented 1 year ago

How might we error out in cases such as the following?

chain
  .blockHash("a peanut butter chicken jelly tuna sandwich")
  .block()

This rune's signature looks like so:

BlockRune<{
  connection: Connection;
  metadata: FrameMetadata;
}, ConnectionError | ServerError | null>

Should we create a new error type? InvalidBlockHashError and unhandle it such that we get the following in the U track?

- ConnectionError | ServerError | null
+ ConnectionError | InvalidBlockHashError | ServerError | null
ryanleecode commented 1 year ago

Would go with MalformedBlockHashError since Invalid could imply you provide a hash that was semantically correct but didnt exist on the chain.

harrysolovay commented 1 year ago

Malformed wouldn't imply not existing on chain either. Perhaps BlockHashDneError?

ryanleecode commented 1 year ago

Malformed wouldn't imply not existing on chain either. Perhaps BlockHashDneError?

We are safe guarding against an invalid string (as per the title) so malformed would be correct as it doesnt imply not existing on chain

ryanleecode commented 1 year ago

Could include both error types but if the block hash doesnt exist (although the hash is semantically well formed) the standard seems to be returning undefined just like storage

harrysolovay commented 1 year ago

How would both errors be useful? I don't forsee different handling between invalid and DNE.

ryanleecode commented 1 year ago

Going back to the original issue, my point was MalformedBlockHashError would be better than InvalidBlockHashError, but this is just from semantic POV.