chore: upgrade oa and oc-verify #544

Nebulis · 2020-08-17T10:59:24Z

needs Open-Attestation/decentralized-renderer-react-components#28

lgtm-com · 2020-08-17T11:07:48Z

This pull request introduces 2 alerts when merging 4a4acf5 into 59c837d - view on LGTM.com

new alerts:

2 for Unused variable, import, function or class

lgtm-com · 2020-08-18T10:16:37Z

This pull request introduces 1 alert when merging 0b980c7 into 6610de1 - view on LGTM.com

new alerts:

1 for Unused variable, import, function or class

gjj · 2020-08-18T11:53:55Z

Open-Attestation/oa-verify#110

lgtm-com · 2020-08-19T03:15:50Z

This pull request introduces 1 alert when merging 22d1b2e into 6610de1 - view on LGTM.com

new alerts:

1 for Unused variable, import, function or class

lgtm-com · 2020-08-19T07:47:00Z

This pull request introduces 1 alert when merging 3bcacfc into 6610de1 - view on LGTM.com

new alerts:

1 for Unused variable, import, function or class

gjj · 2020-08-19T14:22:01Z

OK so I'm done w handling a bad response error returned by Ethers.js (over at Open-Attestation/oa-verify#110)

To test locally

Here's how it looks like. If you want to test,

do so locally with ETHEREUM_PROVIDER=cloudflare NET=mainnet npm run dev
visit https://localhost:3000/?q={"type":"DOCUMENT","payload":{"uri":"https://gist.githubusercontent.com/gjj/1759e1627c562afbdc7a684df23c25de/raw/bbdccc871bcaae8249ad0331942e0b373445e94d/SOR_ALL_2012GCEO2376_00922019-09-02-redacted.opencert","permittedActions":["STORE"]}}
spam browser's back button until you get the 502 bad gateway error

Here's how it'd look like

In the case of a 429 rate limit (caused by user who flood the verifier), or a 502 bad gateway (caused by Cloudflare Ethereum Gateway), we don't know for certain that "This certificate is not valid".

My question

@Nebulis I'm not sure if it's conflicting to say both "certificate is not valid" and "unable to connect to the Ethereum network". People might get confused, what do yall think?

If we want to make "This certificate is not valid" dynamic, problem is that it's static currently... might need a few other changes?

lgtm-com · 2020-08-19T14:25:31Z

This pull request introduces 1 alert when merging d4d3662 into 6610de1 - view on LGTM.com

new alerts:

1 for Unused variable, import, function or class

Nebulis · 2020-08-20T04:58:17Z

@gjj yeah the label certificate is not valid is problematic here let's change that too

testcase still wip

lgtm-com · 2020-08-24T01:46:39Z

This pull request introduces 1 alert when merging d4dc403 into 6610de1 - view on LGTM.com

new alerts:

1 for Unused variable, import, function or class

src/components/CertificateDropZone/Views/UnverifiedView.tsx

lgtm-com · 2020-08-24T02:45:47Z

This pull request introduces 1 alert when merging 3233cb2 into 6610de1 - view on LGTM.com

new alerts:

1 for Unused variable, import, function or class

gjj · 2020-08-24T06:52:09Z

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

Can confirm that missing response is the error message for 429 errors. But bad response is for HTTP status codes < 200 or >= 300

package.json

lgtm-com · 2020-08-24T11:52:54Z

This pull request introduces 1 alert when merging 5dabb6e into 9e243f3 - view on LGTM.com

new alerts:

1 for Unused variable, import, function or class

lgtm-com · 2020-08-24T11:57:36Z

This pull request introduces 1 alert when merging 4593af3 into 9e243f3 - view on LGTM.com

new alerts:

1 for Unused variable, import, function or class

lgtm-com · 2020-08-24T12:06:27Z

This pull request introduces 1 alert when merging f710b90 into 9e243f3 - view on LGTM.com

new alerts:

1 for Unused variable, import, function or class

- i'm not sure why, but somehow it was left out?

lgtm-com · 2020-08-25T18:14:58Z

This pull request introduces 1 alert when merging 9e28d9f into b8ee5c0 - view on LGTM.com

new alerts:

1 for Unused variable, import, function or class

- also refactored some tests - removed OpenAttestationDocument from CertificateViewer (bug with eslint, as it's not picking it up)

- also removed extra ' from some test case names

src/components/tests/download-certificate.spec.js

src/components/Analytics/analytics.test.ts

src/components/CertificateDropZone/Views/UnverifiedView.tsx

package.json

- fixed some mistakes in the test inputs

gjj · 2020-08-31T11:19:14Z

src/components/CertificateDropZone/Views/UnverifiedView.tsx

+      errors.push(TYPES.SERVER_ERROR);
+    } else if (invalidArgument(verificationStatus)) {
+      // if the error is because of the merkleRoot, it should already be handled by the DOCUMENT_INTEGRITY fragment
+      // empty here so that it's not caught by ETHERS_UNHANDLED_ERROR


@Nebulis ok so previously I discovered that merkleRoots that were invalid (odd length, incorrect length/not 64 chars, contains non-hexcadecimal values) were not handled.

Now that it's handled, we expect it to show "Certificate has been tampered with" instead, and that is handled by DOCUMENT_INTEGRITY instead of DOCUMENT_STATUS. If we don't have this, the cases above will get caught by the else block and thus resulting in the "Unhandled error" message.

Not sure what you think about having this check, but nothing is in here? (It should already be handled by this line https://github.com/OpenCerts/opencerts-website/pull/544/files#diff-ad34607bf39b4b3c4414917cf9b807f8R22)

this is for ga right ? at least this could give us mor einformation about what really happened

(explained)

Nebulis · 2020-09-01T06:03:00Z

src/components/CertificateDropZone/Views/UnverifiedView.tsx

-      </span>
+      <div className={css["secondary-links"]}>
+        <span>
+          <Link href=" ">


this will reload the page isnt it ?

changed to not use Link. instead, it's now a span element. text-link css class now includes cursor: pointer

also added button role for the link

Nebulis added the WIP Work in progress by the author label Aug 17, 2020

Nebulis force-pushed the chore/refactor-errors branch 3 times, most recently from ed073c7 to 4fee7bf Compare August 18, 2020 09:54

chore: upgrade oa and oc-verify

0b980c7

Nebulis force-pushed the chore/refactor-errors branch from 4fee7bf to 0b980c7 Compare August 18, 2020 10:12

chore: oc-verify git branch

22d1b2e

feat: handle 5xx errors

3bcacfc

chore: lint and update packages

d4d3662

feat: missing + bad resp error

d4dc403

testcase still wip

fix: remove unhandled error handling for now

3233cb2

gjj reviewed Aug 24, 2020

View reviewed changes

src/components/CertificateDropZone/Views/UnverifiedView.tsx Outdated Show resolved Hide resolved

gjj added 2 commits August 24, 2020 19:44

feat: tests for server error (429, 502)

670376c

fix: lint

5dabb6e

Nebulis commented Aug 24, 2020

View reviewed changes

package.json Outdated Show resolved Hide resolved

fix: testcafe command

4593af3

fix: typos

f710b90

feat: contract not found error, refactor existing errors

9e28d9f

- i'm not sure why, but somehow it was left out?

gjj added 4 commits August 26, 2020 09:56

feat: test case coverage for CONTRACT_NOT_FOUND

846a86a

- also refactored some tests - removed OpenAttestationDocument from CertificateViewer (bug with eslint, as it's not picking it up)

fix: remove Access Control Allow Origin from mock response

5c20913

feat: test coverage for merkle root, normalize path

9df8446

- also removed extra ' from some test case names

feat: test case coverage

ff5ba8c

Nebulis commented Aug 28, 2020

View reviewed changes

src/components/tests/download-certificate.spec.js Outdated Show resolved Hide resolved

fix: use path.join instead of path.normalize

d40b766

gjj removed the WIP Work in progress by the author label Aug 31, 2020

Nebulis commented Aug 31, 2020

View reviewed changes

src/components/Analytics/analytics.test.ts Outdated Show resolved Hide resolved

Nebulis commented Aug 31, 2020

View reviewed changes

src/components/CertificateDropZone/Views/UnverifiedView.tsx Outdated Show resolved Hide resolved

fix: wrong comment, unhandledError block

3cd9202

Nebulis commented Aug 31, 2020

View reviewed changes

package.json Outdated Show resolved Hide resolved

fix: error handling of invalidArgument

329c299

- fixed some mistakes in the test inputs

gjj reviewed Aug 31, 2020

View reviewed changes

Nebulis commented Sep 1, 2020

View reviewed changes

gjj added 3 commits September 1, 2020 16:54

feat: error msg for INVALID_ARGUMENT

3cbf40f

fix: UnverifiedView Link

dddb289

fix: removed the use of preventDefault, added pointer style to text-link

ff56143

also added button role for the link

gjj approved these changes Sep 1, 2020

View reviewed changes

gjj merged commit d9a8038 into master Sep 1, 2020

gjj deleted the chore/refactor-errors branch September 1, 2020 12:44

gjj mentioned this pull request Sep 2, 2020

Handle Ethereum provider errors #517

Closed

gjj mentioned this pull request Sep 28, 2020

Ethers unhandled error when using a random non-OA JSON file #556

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: upgrade oa and oc-verify #544

chore: upgrade oa and oc-verify #544

Nebulis commented Aug 17, 2020

lgtm-com bot commented Aug 17, 2020

lgtm-com bot commented Aug 18, 2020

gjj commented Aug 18, 2020

lgtm-com bot commented Aug 19, 2020

lgtm-com bot commented Aug 19, 2020

gjj commented Aug 19, 2020 •

edited

Loading

lgtm-com bot commented Aug 19, 2020

Nebulis commented Aug 20, 2020

lgtm-com bot commented Aug 24, 2020

lgtm-com bot commented Aug 24, 2020

gjj commented Aug 24, 2020 •

edited

Loading

lgtm-com bot commented Aug 24, 2020

lgtm-com bot commented Aug 24, 2020

lgtm-com bot commented Aug 24, 2020

lgtm-com bot commented Aug 25, 2020

gjj Aug 31, 2020 •

edited

Loading

Nebulis Sep 1, 2020

gjj Sep 1, 2020

Nebulis Sep 1, 2020

gjj Sep 1, 2020

chore: upgrade oa and oc-verify #544

chore: upgrade oa and oc-verify #544

Conversation

Nebulis commented Aug 17, 2020

lgtm-com bot commented Aug 17, 2020

lgtm-com bot commented Aug 18, 2020

gjj commented Aug 18, 2020

lgtm-com bot commented Aug 19, 2020

lgtm-com bot commented Aug 19, 2020

gjj commented Aug 19, 2020 • edited Loading

To test locally

Here's how it'd look like

My question

lgtm-com bot commented Aug 19, 2020

Nebulis commented Aug 20, 2020

lgtm-com bot commented Aug 24, 2020

lgtm-com bot commented Aug 24, 2020

gjj commented Aug 24, 2020 • edited Loading

lgtm-com bot commented Aug 24, 2020

lgtm-com bot commented Aug 24, 2020

lgtm-com bot commented Aug 24, 2020

lgtm-com bot commented Aug 25, 2020

gjj Aug 31, 2020 • edited Loading

Choose a reason for hiding this comment

Nebulis Sep 1, 2020

Choose a reason for hiding this comment

gjj Sep 1, 2020

Choose a reason for hiding this comment

Nebulis Sep 1, 2020

Choose a reason for hiding this comment

gjj Sep 1, 2020

Choose a reason for hiding this comment

gjj commented Aug 19, 2020 •

edited

Loading

gjj commented Aug 24, 2020 •

edited

Loading

gjj Aug 31, 2020 •

edited

Loading