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

exceptions: raise generic exceptions properly #7157

Closed
pared opened this issue Dec 15, 2021 · 3 comments
Closed

exceptions: raise generic exceptions properly #7157

pared opened this issue Dec 15, 2021 · 3 comments
Assignees
Labels
enhancement Enhances DVC

Comments

@pared
Copy link
Contributor

pared commented Dec 15, 2021

In #7120 we had ambiguous error, because the raised FileNotFoundError did not provide any context. I think it would be more informative if we would be raising generic errors with proper messages - like in #7156.

EDIT:
resolving this issue would require finding in the codebase where are we using raise with generic exceptions without any arguments and providing relevant messages.

@pared pared added the enhancement Enhances DVC label Dec 15, 2021
@pared
Copy link
Contributor Author

pared commented Dec 16, 2021

@ykasimov
Copy link

I took a look at this issue and I am posting here my findings.

I used raise (?!NotImplemented|MachineDisabledError|.*from).*(?<!\(|\))$ regex to find exceptions that don’t end with ( or with ( and without from keyword.

It yielded 16 matches in total. all of them seem fine and produce a meaningful message. I reproduced only one (there is a note next to it in the list) and others seem fine from looking at the code.

https://github.com/iterative/dvc/blob/main/dvc/scm.py#L81

https://github.com/iterative/dvc/blob/main/dvc/api/params.py#L238 (used only here with a broad exception: https://github.com/iterative/dvc/blob/main/dvc/utils/__init__.py#L415)

https://github.com/iterative/dvc/blob/main/dvc/cli/parser.py#L100

https://github.com/iterative/dvc/blob/main/dvc/commands/checkout.py#L57

https://github.com/iterative/dvc/blob/main/dvc/commands/stage.py#L90

https://github.com/iterative/dvc/blob/main/dvc/fs/data.py#L67

https://github.com/iterative/dvc/blob/main/dvc/fs/data.py#L71

https://github.com/iterative/dvc/blob/main/dvc/fs/dvc.py#L430 (exception is handled in the calling function with a message)

https://github.com/iterative/dvc/blob/main/dvc/parsing/__init__.py#L89

https://github.com/iterative/dvc/blob/main/dvc/proc/manager.py#L41

https://github.com/iterative/dvc/blob/main/dvc/repo/stage.py#L194

https://github.com/iterative/dvc/blob/main/dvc/repo/experiments/__init__.py#L58

https://github.com/iterative/dvc/blob/main/dvc/repo/experiments/__init__.py#L267

https://github.com/iterative/dvc/blob/main/dvc/utils/serialize/_json.py#L35

https://github.com/iterative/dvc/blob/main/dvc/utils/serialize/_py.py#L131

https://github.com/iterative/dvc/blob/main/dvc/utils/serialize/_py.py#L168

@pared
Copy link
Contributor Author

pared commented Jul 27, 2022

I wonder if those errors didn't migrate to our other projects like dvc-data and dvc-objects. I guess if this issue reoccurs we can reopen. Thanks, @ykasimov for the research!

@pared pared closed this as completed Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhances DVC
Projects
None yet
Development

No branches or pull requests

2 participants