switchupcb / disgo

Create a Discord Bot in Go using this Discord API Wrapper. The next generation of Discord API Consumption.
Apache License 2.0
89 stars 2 forks source link

Issues from DiscordGo #23

Closed switchupcb closed 2 years ago

switchupcb commented 2 years ago

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

switchupcb commented 2 years ago

https://github.com/bwmarrin/discordgo/issues/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 commented 2 years ago

https://github.com/bwmarrin/discordgo/issues/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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

https://github.com/bwmarrin/discordgo/issues/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 commented 2 years ago

https://github.com/bwmarrin/discordgo/issues/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 https://github.com/switchupcb/disgo/issues/22, and will be possible to implement without a refactor.

switchupcb commented 2 years ago

https://github.com/bwmarrin/discordgo/issues/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 commented 2 years ago

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

FedorLap2006 commented 2 years ago

Thank you for outlining some of the older issues though

switchupcb commented 2 years ago

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 commented 2 years ago

The following pull requests are also related to the issues addressed in https://github.com/switchupcb/disgo/issues/23#issuecomment-1196664510.

switchupcb commented 1 year ago

The following issues are solved by using Dasgo.

switchupcb commented 1 year ago

The following issues are avoided by using Automatic Intent Calculation.

switchupcb commented 1 year ago

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

switchupcb commented 1 year ago

@Strum355 So predictable.

a png

Strum355 commented 1 year ago

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