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

Have register throw an error if things didn't work out #41

Open
davidanthoff opened this issue Jul 17, 2020 · 5 comments
Open

Have register throw an error if things didn't work out #41

davidanthoff opened this issue Jul 17, 2020 · 5 comments

Comments

@davidanthoff
Copy link
Contributor

Would it make sense that register throws an error if things didn't work? The content of the .metadata field could just be part of a custom error message?

Right now it is not clear to me what exactly I should check after a call to register to make sure everything worked as intended.

@GunnarFarneback
Copy link
Collaborator

If the metadata field has a key "error" it has failed, as far as I can tell. With the slightly lower level check_and_update_registry_files you can check haserror(status).

@davidanthoff
Copy link
Contributor Author

But why not just throw an error? That seems the standard way in Julia to signal that something didn't work out as planned?

@GunnarFarneback
Copy link
Collaborator

When it's possible I prefer not to have to handle control flow with try-catch. It's not really exceptional to get a registration error.

@davidanthoff
Copy link
Contributor Author

I wouldn't mind something like https://github.com/iamed2/ResultTypes.jl if you don't want to use try catch for this, but the current approach where it is super, super, super easy to just ignore the error strikes me as extremely brittle. It is also very non standard, so it took me for example a long time to figure out what was going on in my code.

I think the core question here is whether we think that a client who calls register should be forced to somehow deal with an error condition. I think they should, the current design makes it way too easy in my mind for errors to just disappear into the nowhere, but things not working as expected.

I actually don't think try catch would be bad here. They are the standard way to do this in Julia, so folks are familiar, they quickly point users to where things go wrong via strack traces etc.

@davidanthoff
Copy link
Contributor Author

Personally, I disagree. I think that using try-catch for control flow is a code smell.

I think packages that are really part of the community should not follow individual contributors idiosyncratic preferences. try catch is well established in Julia.

ResultTypes like solutions work well for functions where the caller needs to look at the return value: if a caller doesn't handle the error case properly, things will crash and errors won't be silently swallowed. But for functions like register, which can be perfectly reasonably used without using the return value, I think throwing errors is by far the most robust approach. It is just way, way too easy to write code like

register()

do_something_else_that_assumes_register_worked()

and then end up with some error deep down inside do_something_else_that_assumes_register_worked that was really caused by the failure in register. Figuring out what is going on in that situation is really difficult, but super easy if an error is thrown. It literally just took me an hour to figure out what is going on in my code because of this situation. If an error had been thrown, this would have been a five minute thing.

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

2 participants