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 Error handling #166

Open
dworthen opened this issue Jan 29, 2024 · 5 comments
Open

Improve Error handling #166

dworthen opened this issue Jan 29, 2024 · 5 comments
Labels

Comments

@dworthen
Copy link
Member

Currently, running g4g cli commands can fail in predictable ways but tends to throw stack traces to stderr that are unintuitive and hard to recover from. Examples of some known exceptions is trying to vote when a user is not a member. In this case I would expect the CLI to repor that user X is not a member of community Y and therefore cannot vote. Another example is trying to deploy a gov4git community for a project url that already has a deployed gov4git community. In that case I would expect the response to indicate that the community already exists and is skipping the deployment. With the current CLI, the approach is try to run a g4g command and if it fails, try running separate validation logic such as checking the community exists and checking user exists and checking user is member through various commands to try and determine the cause of error and recover.

I think we can take some inspiration from HTTP APIs such as GitHub where known errors are returned in the stdout response and only unknown, unrecoverable errors are thrown to stderr. This would require wrapping our current stdout outputs indicating the response status (like a status code) along wiht the response.

For example, a successful panorama command might return

{
    "response": "success",
    "status": 200
    "data": { ...panorama response }
 }

and a caught error might return

{
    "response": "error",
    "status": 401, # Unique codes to identify what the issue was. In this case user is not a member. 
    "message": "User X is not a member of community Y."
}

In the above example I used HTTP status codes as an example but the g4g protocol can come up with unique status codes for the different kinds of responses that are possible. 
@petar
Copy link
Member

petar commented Jan 30, 2024

@dworthen I agree we should transition to a uniform result structure. I've done this in #167

In your example, response and status seem to have an overlapping function. They both partition the space of results into mutually-exclusive named subsets. I opted to use a simpler structure that uses one string status field for this purpose.

Currently, status can only be success or error, but we can add more alternatives as we go.
On success, the result is placed in field result. On error, the error message is placed in field msg.

Here are some example outputs:

$ gov4git --config roadmap_petar.config.json group list --name everybody
{
   "status": "success",
   "returned": [
      "aabwolf",
      "kasiasun",
      "jason-entenmann",
      "dworthen",
      "ctessum",
      "petar",
      "shreyjain13"
   ]
}
gov4git --config roadmap_petar.config.json user prop-get --name pet --key ok
{
   "status": "error",
   "msg": "pet is not a user"
}

Let me know if this looks good to you for a start. Would you want to add anything else?

@petar
Copy link
Member

petar commented Jan 30, 2024

@dworthen #167 resolves the issue with returning a structured result. See the updated PR description for the details #167

Next is your request to return more informative error messages and to handle some circumstances in the backend. For this one, in general, feel free to file individual issues every time you identify an applicable situation.

So far, based on this issue, I created:

@petar
Copy link
Member

petar commented Jan 30, 2024

@dworthen I released the structured result output in https://github.com/gov4git/gov4git/releases/tag/v2.0.4

@dworthen
Copy link
Member Author

Thanks @petar. I think this is a good start. One thing I like about the idea of status codes is fine grain classification of errors which then gives us better recoverability and reporting capabilities. For example, if a user is not a member of a community and has not made a join request then I can try to recover from that state by opening a join request and redirecting the user to the join request status page where they may view the request status. In other cases I may just want to present the error message instead of trying to recover such as in the case the user has made a join request but has been denied. Here are some example error states that come to mind.

  • Invalid or missing fields from the provided config file that are required for current command. Should check formatting or URLs to enforce correct GH URLs and ensure that repos exist and are valid Gov4Git repo with the correct visibility (public/private).
  • User account does not exist in GH.
  • Unable to locate user public/private identity repos.
  • User identity repos are not valid Gov4Git identity repos.
  • User public identity repo is not public.
  • User private identity repo is not private.
  • User is not a member of community and has not made a join request (401)
  • User is not a member of community with a denied join request.
  • User is not authorized to perform current command (403). Think participant trying to run a maintainer command.
  • Underlying project repo does not exist or is not public.
  • Underlying project repo is not a Gov4Git community (there is no corresponding public/private community repos).
  • Community public/private repo are not valid Gov4Git community repos.

@dworthen
Copy link
Member Author

dworthen commented Feb 4, 2024

A few more error responses

  • api.github.com unreachable. Service may be down.

Sometimes there is a delay in creating things in github and them showing up in the API/protocol. For example when I start with a brand new user and log in for the first time it creates their personal identity repos and initializes them with the init id command. I can view the repo in GitHub but Gov4Git commands will fail with a stack trace indicating it cannot find the public identity repo. That "repository not found". So for API requests that may have a delay it would be great to get an error stating the problem but also indicating that if you are a new user it may take some time for the system to sync and to try again later.

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

No branches or pull requests

2 participants