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

Issues from DiscordGo #23

Closed
switchupcb opened this issue Jul 25, 2022 · 16 comments
Closed

Issues from DiscordGo #23

switchupcb opened this issue Jul 25, 2022 · 16 comments
Labels
documentation Improvements or additions to documentation

Comments

@switchupcb
Copy link
Owner

Disgo solves the following issues from https://github.com/bwmarrin/discordgo/issues.

@switchupcb switchupcb added the documentation Improvements or additions to documentation label Jul 25, 2022
@switchupcb
Copy link
Owner Author

bwmarrin/discordgo#513 (by @ErikMcClure) outlines a potential deadlock scenario that involves the calling of multiple Connect() and Disconnect() equivalent-calls while a Opcode 11 Heartbeat ACK is received.

Disgo avoids this issue through multiple mechanisms:

  1. Only one connect action can occur at at any given time through the usage the Session's only mutex.
  2. A heartbeat will only be sent to a channel (via s.heartbeat.send) when another goroutine is listening to that channel (s.heartbeat.send) due to the manager mechanism (which prevents the heartbeat goroutine from closing BEFORE goroutines that can generate heartbeats).
  3. This is not highlighted on the golangci-linter, while this user's alert of this issue occurs from that same linter.
  4. We use a test to connect, disconnect, and reconnect to the API with -race.

@switchupcb
Copy link
Owner Author

bwmarrin/discordgo#544 outlines the possibility for Discord to send an Opcode 9 Invalid Session (which warrants a re-identify) upon a reconnection attempt. This is handled by Disgo in the session.go initial function.

@switchupcb
Copy link
Owner Author

The following (open) issues have occurred due to an incorrect type definition or non-existent type definition in relation to JSON tags:

Disgo avoids these issues through the usage of Dasgo. In short, @bwmarrin and @FedorLap2006 may benefit from using Dasgo to stay up-to-date while reducing maintenance load.

@switchupcb
Copy link
Owner Author

switchupcb commented Jul 27, 2022

The following open issues have occurred due to an incorrect type definition or non-existent type definition:

There are 8 open issues and 37 closed issues with the query missing.

Disgo avoids these issues through the usage of Dasgo. In short, @bwmarrin and @FedorLap2006 may benefit from using Dasgo to stay up-to-date while reducing maintenance load.

@switchupcb
Copy link
Owner Author

bwmarrin/discordgo#649 (by @UlisseMini) outlines a concern with the New function in discordgo. While this isn't inherently an issue, it abstracts how the client is being initialized. In actuality, New initializes a session which represents a "bot" client. This abstraction can be confusing since a WebSocket Session isn't actually required to send requests, and there is no guarantee that the user needs an initialized "WebSocket Session".

Disgo avoids this confusion by being explicit in its idiomatic initialization, at the cost of a few extra lines.

@switchupcb
Copy link
Owner Author

bwmarrin/discordgo#712 (by @riking) outlines an issue with the ratelimiting functionality of DiscordGo. Specifically, a per-resource per-route rate limit issue: This is being addressed by Disgo in #22, and will be possible to implement without a refactor.

@switchupcb
Copy link
Owner Author

bwmarrin/discordgo#966 (by @sntran) outlines an issue with a lack of support for the OAuth2 flow Client Credentials Grant. Disgo implements and provides steps for all OAuth2 flows in oauth2.go.

@FedorLap2006
Copy link

We're currently working on getting the types up to date, in particular partial and null fields.

@FedorLap2006
Copy link

Thank you for outlining some of the older issues though

@switchupcb
Copy link
Owner Author

We're currently working on getting the types up to date, in particular partial and null fields.
@FedorLap2006

Great! If you haven't already, you can consider using Dasgo. We realized that every Go Discord API Wrapper was having trouble with missing fields, or incorrectly defined fields in relation to JSON. In order to solve this issue, we created a Go API Types Library for the Discord API called Dasgo. This library is built with a specification and is not implementation specific to Disgo; meaning that other Go API Wrappers can use it for their types. If Dasgo is used by many API Wrappers, we can unite the entire Discord Go community while still allowing multiple API Wrappers to exist.

Dasgo has more benefits beyond type definitions. As an example, discordgo's wsapi.go L112 features a magic number "10". In actuality, this number refers to an Opcode 10 Hello which is represented in Dasgo by FlagGatewayOpcodeHello. This occurs again at wsapi.go L127` (Dispatch), 140 (Resume), and even for strings such as L179. If Dasgo was used here, the reader of the code would be able to clearly identify the reason for the literal number or string that is used.

Dasgo doesn't have to be used as a dependency, and can be derived from code generation (as shown by Disgo). If you need assistance with that, feel free to let me know! With more people focusing on Dasgo to fix changes like these, maintainers will have less work to do when Discord rolls out an inevitable change.

@switchupcb
Copy link
Owner Author

The following issues are solved by using Dasgo.

@switchupcb
Copy link
Owner Author

The following issues are avoided by using Automatic Intent Calculation.

@switchupcb
Copy link
Owner Author

@FedorLap2006, join the Disgo side. We have cookies... Strictly necessary of course.

@switchupcb
Copy link
Owner Author

switchupcb commented Dec 5, 2022

@Strum355 So predictable.

a png

@Strum355
Copy link

Strum355 commented Dec 5, 2022

thanks for the content, looks great in a github platform abuse report 🙂

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

No branches or pull requests

3 participants