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 #988

Closed
dbrumann opened this issue Sep 23, 2022 · 2 comments · Fixed by #1079
Closed

Improve error handling #988

dbrumann opened this issue Sep 23, 2022 · 2 comments · Fixed by #1079

Comments

@dbrumann
Copy link
Collaborator

I would like to make raising and handling exceptions/errors more user friendly. What this means is, that we shouldn't rely on ShouldNotHappenException quite as often, e.g. in Qossmic\Deptrac\Supportive\Console\Application::doRun a RuntimeException would probably make more sense. In any case, the exception should contain a message that is helpful to users of Deptrac, when it is thrown. In the example above a message like "Could not determine current working directory. Please check if any one of the parent directories does not have the readable or search mode set. See chmod for more information on modes and permissions.". While this is rather verbose, it will tell users what went wrong and guide them towards a solution, if possible.

For future extension authors it would be helpful to be able to catch any of our exceptions, which is why we have the ExceptionInterface. As far as I can tell, we already use this consistently, so I don't think we need to do anything here, but if we missed implementing the interface somewhere, we should add it. Similarly raising exceptions should be consistent, so all of our exceptions should use static factory methods like Qossmic\Deptrac\Supportive\File\Exception\FileCannotBeParsedAsYamlException::fromFilenameAndException. There are a few exceptions to this rule currently that need to be updated, e.g. Qossmic\Deptrac\Supportive\File\Exception\FileNotExistsException

Tasks

  • Clean up unused exceptions
  • Ensure all custom exceptions implement ExceptionInterface
  • Ensure all custom exceptions use a static factory method
  • Replace overly generic or vague exceptions with dedicated exceptions or better fitting ones, especially ShouldNotHappenException
  • Add helpful error messages to raised exception
  • Declare which exceptions are expected to be thrown using @throws in the doc block
@patrickkusebauch
Copy link
Collaborator

I would also want to see the checks being enabled in psalm nad PHPStan for proper exception handling.

@patrickkusebauch
Copy link
Collaborator

Picking this one up.

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

Successfully merging a pull request may close this issue.

2 participants