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

Improvement: Ensure contract returns meaningful errors #68

Closed
2 tasks
carlos-kryha opened this issue Oct 20, 2023 · 1 comment · Fixed by #69
Closed
2 tasks

Improvement: Ensure contract returns meaningful errors #68

carlos-kryha opened this issue Oct 20, 2023 · 1 comment · Fixed by #69
Assignees

Comments

@carlos-kryha
Copy link
Contributor

Description

Contract errors thrown via assert.fail(<error>); cannot be inspected from the frontend using makeOffer error callbacks. The following code is responsible for catching errors on the client side (mint character in this case):

service.makeOffer(spec, proposal, offerArgs, ({ status, data }: { status: string; data: object }) => {
    if (status === "error") {
      console.error("Offer error", data, typeof data);
      if(errorCallback) errorCallback(JSON.stringify(data));
    }

Depending on where and how the error is thrown, we get 3 kinds of errors from makeOffer:

  1. wallet throws: if we attempt to use more funds than the account owns, we receive an error detailing the problem
  2. Contract throws via @agoric/store mustMatch. in this case we get an error in the format of mint.ts:36 Offer error Error: offerArgs: (an object) - Must have missing properties ["name"] string
mustMatch(
  offerArgs,
  M.splitRecord({
    name: M.string(harden({ stringLengthLimit: 20 })),
  }),
  'offerArgs',
);
  1. Contract throws via assert.fail(<error>); resulting in the following error mint.ts:36 Offer error Error: (a string) string

Cases 1 and 2 work as intented and return meaningful errors, 3 does not.

Repro

Modify this line to use a name which has already been used, this should triggered scenario 3.

Acceptance Criteria

  • All instances of assert.fail(<error>) can be used to get meaningful errors using makeOffer or they have been replaced with something that can
  • QA
@Pandelissym
Copy link
Contributor

Digging around a bit I found that this might be a reason why our existing error messages are not being returned to the frontend. The details tag (X tag) we prepend our strings with hides all data and only displays it in the logs. Removing the X tag or using the quote tag might help with this issue.

 * @typedef {(template: TemplateStringsArray | string[], ...args: any) => DetailsToken} DetailsTag
 *
 * Use the `details` function as a template literal tag to create
 * informative error messages. The assertion functions take such messages
 * as optional arguments:
 * ```js
 * assert(sky.isBlue(), details`${sky.color} should be "blue"`);
 * ```
 * or following the normal convention to locally rename `details` to `X`
 * and `quote` to `q` like `const { details: X, quote: q } = assert;`:
 * ```js
 * assert(sky.isBlue(), X`${sky.color} should be "blue"`);
 * ```
 * However, note that in most cases it is preferable to instead use the `Fail`
 * template literal tag (which has the same input signature as `details`
 * but automatically creates and throws an error):
 * ```js
 * sky.isBlue() || Fail`${sky.color} should be "blue"`;
 * ```
 *
 * The details template tag returns a `DetailsToken` object that can print
 * itself with the formatted message in two ways.
 * It will report full details to the console, but
 * mask embedded substitution values with their typeof information in the thrown error
 * to prevent revealing secrets up the exceptional path. In the example
 * above, the thrown error may reveal only that `sky.color` is a string,
 * whereas the same diagnostic printed to the console reveals that the
 * sky was green. This masking can be disabled for an individual substitution value
 * using `quote`.
 *
 * The `raw` property of an input template array is ignored, so a simple
 * array of strings may be provided directly.
 */```

@Pandelissym Pandelissym self-assigned this Oct 23, 2023
Pandelissym added a commit that referenced this issue Oct 24, 2023
- Removed the `X` tag from error messages in the contract. This ensures
the error message is also returned to the makeOffer call. Now we can
match on the error message and display an appropriate error to the user.

closes #68

Co-authored-by: Pandelis Symeonidis <pandelis@kryha.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants