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

Improve response messages and http status codes in admin API #521

Closed
nrempel opened this issue May 21, 2020 · 7 comments
Closed

Improve response messages and http status codes in admin API #521

nrempel opened this issue May 21, 2020 · 7 comments
Assignees

Comments

@nrempel
Copy link
Contributor

nrempel commented May 21, 2020

With regards to aca-py error handling I've created a couple tickets as examples of the kinds of cases we should handle better:

#520
#519
#518

These are just examples though. These sorts of cases are all over the place. I don't think it's worth writing them all up as tickets. Some cases are responses to admin API requests that can return more useful information and appropriate status codes. Others might be related to how the server has been run (with or without a ledger connection, for example). It should be possible to run aca-py without a ledger connection, but we should gate access to particular functionality better.

@andrewwhitehead
Copy link
Contributor

Status code handling has been improved but we might still want some changes around missing ledger connections

@swcurran swcurran assigned ianco and unassigned ianco Sep 30, 2020
@ianco ianco self-assigned this Sep 30, 2020
@ianco
Copy link
Contributor

ianco commented Sep 30, 2020

Looking through some of Nick's tickets it seems that the startup parameters need to be reviewed as well.

E.g. should "--wallet indy" be required? Should "--endpoint" be required?

I don't know if it makes sense to run an aca-py agent without some of these parameters specified

@ianco
Copy link
Contributor

ianco commented Sep 30, 2020

In general the API's should return:

  • HTTP 400 (with an error message) for errors on input parameters (user can retry with different parameters)
  • HTTP 404 if a record is expected and not found (generally for GET requests that fetch a single record)
  • HTTP 500 if there is some other processing error (i.e. won't make any difference what parameters the user tries) with an error message

.. and should not return:

  • HTTP 500 with a stack trace (we should handle error conditions with a 400 or 404 response, and catch errors and provide a meaningful error message)

@swcurran
Copy link
Contributor

I definitely think we need a better way to document what is needed. The plan of going to a config file would really help with that -- here are the recommended settings, and a doc section on why some options would/would not be needed.

@ianco
Copy link
Contributor

ianco commented Sep 30, 2020

aca-py start: error: the following arguments are required: -it/--inbound-transport, -ot/--outbound-transport

What can the agent do with this minimal set of parameters?

There are basically 4 roles we can run an agent: Issuer, Holder/Prover, Verifier, Router/Mediator

Correct?

The first 3 (currently) require an Indy ledger (and therefore an Indy wallet), I don't think Router/Mediator does ...

@ianco
Copy link
Contributor

ianco commented Sep 30, 2020

In terms of startup parameters, I suggest when running "aca-py start":

  • Endpoint should be required ("-e" or "--endpoint")
  • Ledger should be required ("--genesis-transactions", "--genesis-file" or "--genesis-url")
  • Wallet configuration should be required, and should default to "indy"

(Inbound and outbound transports are already required)

If we want to run with no ledger we should have a "--no-ledger" parameter. Likewise if there is a "no wallet" scenario we should add a "--no-wallet" parameter

When running "aca-py provision" both Ledger and Wallet should be required. (Transports and endpoint are not required.)

@ianco
Copy link
Contributor

ianco commented Oct 2, 2020

Adding some API development standards to https://github.com/hyperledger/aries-cloudagent-python/blob/master/AdminAPI.md before doing any API updates.

@ianco ianco closed this as completed Sep 1, 2021
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

No branches or pull requests

4 participants