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

Attestation service responses and exceptions #48

Closed ambertch closed 6 years ago

ambertch commented 6 years ago

Checklist:

Description:

Please explain the changes you made here:

Implements #29, #33, #42, #40:

In addition,

ambertch commented 6 years ago

One inconsistency with develop (which probably should be resolved before this PR is merged) is that #29 was implemented with exceptions having an string rather than dictionary value, so handle_request now returns two error responses:

  1. In the event of an exception being raised in VerificationService (an array of strings): https://github.com/OriginProtocol/bridge-server/pull/48/commits/980d61822c2c6e3e5e8f119392244cd9cdb68b2e#diff-c6a86d88d4862dc3dc261772dc68e5daR41)
  2. In the event of a validation error from Marshmallow (an array of dictionaries): https://github.com/OriginProtocol/bridge-server/blob/develop/api/helpers.py#L26

A dapp developer calling the attestation service endpoints will get a different error response format returned depending on whether they made the error-causing request with bad params, or the request hit an exception.

I wasn't sure if the [code/path/message] format that was originally implemented was meant for filtering on, and if it's ok to use string error messages (the errors could also be filtered based on class type).

If so, returning the errors as strings would change a request that fails Marshmallow validation from

{
    "errors": [
        {
            "code": "INVALID_REQUEST",
            "path": "code",
            "message": "Missing data for required field."
        },
        {
            "code": "INVALID_REQUEST",
            "path": "email",
            "message": "Not a valid email address."
        },
        {
            "code": "INVALID_REQUEST",
            "path": "identity",
            "message": "Not a valid string."
        }
    ]
}

to something like

{
    "errors": ["code - Missing data for required field.",        
               "email - Not a valid email address.",
               "identity - Not a valid string."
    ]
}

Feedback appreciated!

ambertch commented 6 years ago

api/README has been updated

tyleryasaka commented 6 years ago

@ambertch Looking good! I can approve once the conflicts have been resolved.

tyleryasaka commented 6 years ago

Thanks for all the work here - it's looking good. Tomorrow I'm going to resolve the conflicts in this PR and then make sure everything works with the rest of the stack - origin-js and demo-dapp. That's a to-do for me. Sorry for the delay on this.