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

feat: new error handling #110

Merged
merged 20 commits into from
Aug 31, 2020
Merged

feat: new error handling #110

merged 20 commits into from
Aug 31, 2020

Conversation

gjj
Copy link
Contributor

@gjj gjj commented Aug 18, 2020

Purpose

This MR attempts to fix the issues highlighted in OpenCerts/opencerts-website#517:

  • Cloudflare Ethereum Gateway's random 502 Bad Gateway errors (something we can't control)
  • Infura and Cloudflare's 429 rate limit error, caused by the user while spamming the verifier

For reference,

A 4xx code indicates an error caused by the user, whereas 5xx codes tell the client that they did everything correctly and it's the server itself who caused the problem.

In addition to the above, we also introduced error handling for INVALID_ARGUMENT type of errors.

Decision

We decided to check Ethers error codes instead of its error messages for the two new error checks introduced in this MR.

This means that:

  • SERVER_ERROR encompasses { "bad response", "failed response", "missing response" }
    • Instead of checking for Ethers error messages (e.g. bad response, missing response, etc.), we now group them using Ethers error codes wherever possible
    • Rationale is that we don't need a fine level of granularity at this point in time (i.e. we don't need to know that it's 4xx vs 5xx
  • INVALID_ARGUMENT encompasses a few funky errors that we didn't catch before like { "incorrect data length", "hex is of odd-length" } as explained below:
    • 61dc9186345e05cc2ae53dc72af880a3b66e2fa7983feaa6254d1518540de5 is even length, but is not 64 characters (or 32 bytes) as required of merkle roots - Ethers returns error message incorrect data length
    • 61dc9186345e05cc2ae53dc72af880a3b66e2fa7983feaa6254d1518540de50 is not of valid length - Ethers returns error message hex is of odd-length
    • 61dc9186345e05cc2ae53dc72af880a3b66e2fa7983feaa6254d1518540de50a is OK, leaving this here for an example of a valid merkleRoot to provide context for the two errors above

This MR also includes the testcases for:

  • server error
  • invalid argument

Finally, moved common/smartContract/documentStoreErrors.tssrc/verifiers/documentStoreStatus/errors.ts since Token Registry follows the same structure.

For future ref

HTTP-related errors from Ethers: https://github.com/ethers-io/ethers.js/blob/9640e864a68b4a9e84e820f0ceaf1eb56c66715f/packages/web/src.ts/index.ts

Bytes-related errors from Ethers: https://github.com/ethers-io/ethers.js/blob/a78ca7eb8d841f8dbbdda13ef8b128a8817d10a4/packages/bytes/src.ts/index.ts

tl;dr there are just simply too many error messages to handle, so handling error codes (wherever possible) is a better choice in this case

@gjj gjj requested a review from Nebulis August 27, 2020 10:12
@gjj gjj removed the WIP label Aug 27, 2020
package.json Outdated Show resolved Hide resolved
@gjj gjj merged commit 5d301f6 into master Aug 31, 2020
@gjj gjj deleted the feat/new-errors branch August 31, 2020 08:26
@john-dot-oa
Copy link
Contributor

🎉 This PR is included in version 4.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

3 participants