OriginProtocol / origin-bridge

We've moved to a monorepo: https://github.com/OriginProtocol/origin/tree/master/infra/bridge
MIT License
15 stars 7 forks source link

Sparrow dom/attestation error handling #114

Closed sparrowDom closed 6 years ago

sparrowDom commented 6 years ago

Checklist:

Description:

Adding additional error handling to airbnb attestation

sparrowDom commented 6 years ago

@tomlinton can you review this again please?

tomlinton commented 6 years ago

@sparrowDom will test it shortly!

sparrowDom commented 6 years ago

@tomlinton thanks.

I don't have the permission to merge this. Probably @micahalcorn will merge it when he reviews https://github.com/OriginProtocol/origin-dapp/pull/303 that depends on this Pull Request.

sparrowDom commented 6 years ago

@tomlinton I have added 2 commits that change email validation errors to a general error: https://github.com/OriginProtocol/origin-bridge/pull/114/commits/d95b28665df500502b8d5c37c48e3c802d87622e https://github.com/OriginProtocol/origin-bridge/pull/114/commits/c4bbd629e51d70200ed935da23e61b6efde41730

Reason for the first change is that in second stage of email attestation only code field is displayed and email field is not and users wouldn't be able to see the error message tied to email field.

The second was a validation error, and I think a general service error would be more fitting.

Can you check if the changes make sense to you?

micahalcorn commented 6 years ago

I seem to be getting Could not send verification code. Please try again shortly. when I try to call email/generate-code using the same environment variables that worked for me yesterday. SendGrid is showing everything operational on their public status. Is there any way to debug this without checking the logs in our account?

sparrowDom commented 6 years ago

Hey yes, can you check in your development console in browser what bridge server the requests are being issued to, and what is the response?

In my case the request goes to http://localhost:5000/api/attestations/email/generate-code with POST method, and response is an empty object with status code 200: screenshot 2018-08-03 22 37 41

sparrowDom commented 6 years ago

And on bridge server in environmental variables file .enva variable SENDGRID_API_KEY is set to SG.4....KzKg. Have just tried it this second and it works for me.

micahalcorn commented 6 years ago

Thanks guys, false alarm. I somehow had a rogue character creep into my API key, so it was failing with a 401 under the hood.

@tomlinton, should we at least check the error message here and provide something more meaningful in our EmailVerificationError?

tomlinton commented 6 years ago

@micahalcorn I think we do definitely need a logging implementation in origin-bridge so the errors from SendGrid/other attestation providers are logged on the bridge server (and probably something like Sentry so we get alerts). :thumbsup: That general error is for displaying in origin-dapp because we don't want to be displaying errors about bad API Keys/origin-bridge configuration issues in the dapp.

micahalcorn commented 6 years ago

@tomlinton oh, right, because the key is stored on the bridge server and not passed in from origin-js. Got it, thx. 👌

sparrowDom commented 6 years ago

Yeah logging would be really useful for development debugging and also for keeping track what goes on on staging and production servers.