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

Errors in parameter getters / setters are not reported #280

Open
ettersi opened this issue Apr 21, 2023 · 5 comments
Open

Errors in parameter getters / setters are not reported #280

ettersi opened this issue Apr 21, 2023 · 5 comments
Assignees

Comments

@ettersi
Copy link
Collaborator

ettersi commented Apr 21, 2023

The current implementation does not report errors raised in parameter getter and setters.

I believe the culprit is this generic catch clause here.

@notthetup
Copy link
Collaborator

Maybe we should log some kind of errors but not others?

@mchitre
Copy link
Member

mchitre commented Apr 21, 2023

Values passed in to setters can come from users and can be meaningless in the context. If we insisted on every setter fully validating the inputs, the code would become verbose and less readable. We need a middle ground on how to handle such inputs.

The current implementation is very lax, but simply ignoring errors. The other extreme would be to log stack traces as errors. That feels alarmist and will make logs harder to work with. Perhaps we can log them as single liner warnings?

@notthetup
Copy link
Collaborator

Maybe we can ignore data conversion type errors but log other types of errors?

@ettersi
Copy link
Collaborator Author

ettersi commented Apr 21, 2023

I'm not following what is the link between validation and error reporting. Are we talking about conversion errors like the user passes a string when an integer is expected? Errors like these should be fairly easy to detect and ignore, like @notthetup suggested. It seems reasonable to me that anything beyond that would qualify as a programming error. Otherwise, you'd e.g. get an error message if you make a programming error in a processrequest(), but you wouldn't get one if you moved exactly the same code to a setter.

@mchitre mchitre self-assigned this Apr 22, 2023
@mchitre
Copy link
Member

mchitre commented Apr 22, 2023

Let's do warnings for conversion errors, and stack traces for other exceptions. And if that gets noisy in the logs then revisit why.

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

No branches or pull requests

3 participants