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

Deserialization of discord template into DiscordTemplate struct fails #1394

Open
merlinfuchs opened this issue Jun 22, 2023 · 4 comments · May be fixed by #1482
Open

Deserialization of discord template into DiscordTemplate struct fails #1394

merlinfuchs opened this issue Jun 22, 2023 · 4 comments · May be fixed by #1482
Labels
fix Pull requests and issues related to bug fixes and structural inconsistencies structs API structs and constants related changes

Comments

@merlinfuchs
Copy link
Contributor

merlinfuchs commented Jun 22, 2023

Deserializing a discord template into the DiscordTemplate struct doesn't work as it's using the standard Guild struct for the serialized_source_guild field. See

SerializedSourceGuild *Guild `json:"serialized_source_guild"`

This fails because all IDs in the discord template data are integers instead of strings.

@FedorLap2006 FedorLap2006 added fix Pull requests and issues related to bug fixes and structural inconsistencies structs API structs and constants related changes labels Aug 31, 2023
@kagadar
Copy link

kagadar commented Jan 10, 2024

Since this is a bug with the Discord API (snowflakes must always be returned as strings in the HTTP API), I have created an issue on their side: discord/discord-api-docs#6609

In the meantime, discordgo may want to consider using json.Number as the type for snowflake fields, since json.Unmarshal is capable of decoding both numbers and strings into fields of this type. Other fixes may resut in runtime errors if/when Discord fixes the return type in the API.

@merlinfuchs
Copy link
Contributor Author

merlinfuchs commented Jan 10, 2024

This has always been the case for templates, so I honestly don't think it's a bug. The snowflakes in guild templates are just serial numbers starting at 0. Changing it now would be a breaking change in the Discord API which won't happen until the next major version.
Let's wait for a response from Discord staff on the issue tho

@kagadar
Copy link

kagadar commented Jan 10, 2024

This has always been the case for templates, so I honestly don't think it's a bug. The snowflakes in guild templates are just serial numbers starting at 0. Changing it now would be a breaking change in the Discord API which won't happen until the next major version. Let's wait for a response from Discord staff on the issue tho

I agree it would be a breaking change, but it's still a bug, since the field is snowflake:

Because Snowflake IDs are up to 64 bits in size (e.g. a uint64), they are always returned as strings in the HTTP API to prevent integer overflows in some languages

Returning an int breaks the invariant that snowflake is always a string.

I don't know what the policy on breaking changes is in this project, but I'll have a look into changing snowflake fields here to json.Number.

@FedorLap2006
Copy link
Collaborator

FedorLap2006 commented Feb 16, 2024

Returning an int breaks the invariant that snowflake is always a string.

I don't know what the policy on breaking changes is in this project, but I'll have a look into changing snowflake fields here to json.Number.

I don't think we need to update all snowflake fields to support number values. Because these fields aren't actually snowflakes (see the attachment) and this behavior occurs only in the serialized guild object.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Pull requests and issues related to bug fixes and structural inconsistencies structs API structs and constants related changes
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants