Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better error messages for predicate setData #1552

Open
arboleya opened this issue Dec 19, 2023 · 8 comments
Open

Better error messages for predicate setData #1552

arboleya opened this issue Dec 19, 2023 · 8 comments
Labels

Comments

@arboleya
Copy link
Member

The current encoding error messages were weak on details and should be improved.

@arboleya arboleya added this to the 2 - Beetle milestone Jan 8, 2024
@nedsalk nedsalk self-assigned this Jan 14, 2024
@nedsalk
Copy link
Contributor

nedsalk commented Jan 18, 2024

One approach to do this is would be to make each coder aware of the abi it's encoding/decoding so that they can throw an abi-specific message like: encode-error: invalid u8 for property "a.b.c.d: passed in value is 24242, max is 255" or, to be fancier:

encode-error: invalid u8 while encoding abi:
{
  a: {
  a1: 54,
    b: {
      b1: "text",
      b2: "0xasdfwer",
      c: {
       c1: [1,2,3],
-----> d: Encoding 6464, max for u8 is 255.
      }
    }
  }
}

It would definitely be nice to have one of the above, but is it worthwhile to implement? On first hunch it feels like it would require a lot of custom code.

I don't see another way to substantially improve the error messages besides this.

@FuelLabs/sdk-ts What do you think?

@nedsalk
Copy link
Contributor

nedsalk commented Jan 18, 2024

@arboleya It might be wise to block this until the new encoding scheme is finished.

@arboleya arboleya modified the milestones: 2 - Beetle, 3 - Wasp Jan 31, 2024
@arboleya
Copy link
Member Author

arboleya commented Feb 5, 2024

Blocked by:

@nedsalk nedsalk removed their assignment Feb 22, 2024
@arboleya
Copy link
Member Author

@nedsalk @Dhaiwat10 Do you guys think this is still blocked, now that we merged #1826?

cc @petertonysmith94

@petertonysmith94
Copy link
Contributor

@arboleya I believe it's blocked by the new encoding #1672, rather than #1826.

In light of #1826, the problem will be on Predicate instantiation - rather than the setData method (which has been removed).

Maybe @Dhaiwat10 could confirm my understanding 🙏🏼

@arboleya
Copy link
Member Author

I wonder if these errors weren't revert errors, in which case could be related to #1840.

@petertonysmith94
Copy link
Contributor

I believe this issue is in reference to the Coder based errors in general. They're super generic (in description) and I don't believe there is any feedback to where or how these errors have been thrown (I could be wrong).

Take for instance the ArrayCoder, wouldn't the index of the Array be useful.

Also the aggregate coders, take TransactionScriptCoder for instance, would be good to know the argument that failed - rather than Invalid u8. when an error occurs for the witnessCount.

@arboleya
Copy link
Member Author

This feedback came in response to the development of the WebAuthn, referenced in the issue description:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants