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

fix(Activity): accept non-string application ids #1513

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

TomWright
Copy link

@TomWright TomWright commented Mar 28, 2024

I've been writing some code to read VoiceStates on application start-up and noticed it wasn't finding records where I would expect.

I noticed this coincided with the error wsapi.go:646:onEvent() error unmarshalling GUILD_CREATE event, json: cannot unmarshal number into Go struct field Presence.presences.activities of type string.

After doing some digging, I realised that discord is sending along activities with an application_id of 1 in some cases (Note that is 1 the integer, not "1" the string)

This causes the GUILD_CREATE event (and others) to fail parsing and not get added to the state, meaning I then have no way of accessing the current voice states when my app starts up.

I have run my app against my discordgo fork and it is now working as expected.

I know this logic isn't ideal, but I don't know of a better way to fix it given:

  1. We don't want to break the API.
  2. I don't understand the scenarios in which discord would be returning a number instead of a string.

Example activity with a bad app id:

{
          "url": "https://www.youtube.com/watch?v=dQw4w9WgXcQ",
          "type": 1,
          "timestamps": {
            "start": 1711557036583
          },
          "state": "Playing Single Player",
          "session_id": "cedf635683339917672bc194bf59b35d",
          "name": "Grand Theft Auto 6",
          "id": "876e03ca62d9cfc2",
          "flags": 1,
          "details": "Grand Theft Auto VI",
          "created_at": 1711557037898,
          "assets": {
            "small_image": "mp:external/ruoyEd5pgWX_V3GB4FWCzMZyRVJZFedojHrb07B-nJA/https/media.tenor.com/ByYRHA88z_8AAAAj/bottas-crash.gif",
            "large_image": "mp:attachments/812394872392122390/1218155883431858207/gtavi.png?ex=6606a312&is=65f42e12&hm=8a29b83a446c91ea04500d265ee317709a0c3aff971e04807ee48e1b2b3b5da2&"
          },
          "application_id": 1
        }

Out of 1142 activities found on this GUILD_CREATE event, the above is the single activity with a non-string application_id.

@TomWright
Copy link
Author

Related to:

It seems as though a numeric response for snowflake types is intended. A better fix may be to have a Snowflake type that deals with this, although that is a breaking change.

structs.go Outdated
@@ -2174,7 +2174,7 @@ func (activity *Activity) UnmarshalJSON(b []byte) error {
Type ActivityType `json:"type"`
URL string `json:"url,omitempty"`
CreatedAt int64 `json:"created_at"`
ApplicationID string `json:"application_id,omitempty"`
ApplicationID interface{} `json:"application_id,omitempty"`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's only an integer number, I think it'd be better to use json.Number for this

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good shout - I've just made that change

wsapi.go Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While we should fix this at some point, these changes are unrelated, please remove

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fair. My IDE is auto formatting here

@FedorLap2006 FedorLap2006 changed the title Fix error unmarshalling GUILD_CREATE event fix(Activity): accept non-string application ids Apr 1, 2024
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

Successfully merging this pull request may close these issues.

None yet

2 participants