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

Make enum/flags strict when deserializing #1994

Open
1 task done
davfsa opened this issue Aug 4, 2024 · 2 comments
Open
1 task done

Make enum/flags strict when deserializing #1994

davfsa opened this issue Aug 4, 2024 · 2 comments
Labels
enhancement New feature or request

Comments

@davfsa
Copy link
Member

davfsa commented Aug 4, 2024

Summary

Remove the following lines to error on failure instead of returning what was cast in

Involves changing

def __call__(cls, value: typing.Any) -> typing.Any:
"""Cast a value to the enum, returning the raw value that was passed if value not found."""
return cls._value_to_member_map_.get(value, value)

and Flag to ignore unknown bits, as well as making sure that the new errors are properly handled and/or ignored

Why is this needed?

Would provide a nicer interface for developers and make it easier to catch deserialization errors (like #1992), as currently we just return what was cast in when we are unable to deserialize it properly

Ideal implementation

typing.Union will need to be removed from types that allow an enum/flag and switch to only allowing the enum type.

Checklist

  • I have searched the issue tracker and have made sure it's not a duplicate. If it is a follow up of another issue, I have specified it.
@davfsa davfsa added the enhancement New feature or request label Aug 4, 2024
@syncblaze
Copy link
Contributor

When im finished with #2005 i can take a look at that and make a PR. If someone wants to do it before me feel free to go ahed.

@syncblaze
Copy link
Contributor

syncblaze commented Aug 7, 2024

I was thinking about this issue for the past few minutes and it doesnt feel quite right. I get that deserialization errors like #1992 should be prevented, but that doesnt feel like the best option.

When discord adds new AuditLogActionTypes (for example) or basicly adds anything to any enum that we have, this could result into an error as you said. Then you would need to handle this error and set the field to None? or do you pass the error through to the user and let the event for example fail or not event trigger?

This would put more stress onto the hikari contributers because they would have even more reasons to push out the newest features asap.

With the implemantation rn unknown values in emus and flags are just passed to the field as their value instead of the enum/flag, that means that hikari users who maybe are already getting into touch with not added features (for example if they are listining to AuditLogEntryCreate event, which also triggers for AuditLogEntryActionType's who are not added yet) can already partly work on them and dont expierience unexpected behaviour.

Maybe i am thinking into the wrong direction, i would be happy to hear other opinions or other ways of handling the resulting error.

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

No branches or pull requests

2 participants