Port Discord RPC from rich-go to go-discordrpc#73
Merged
Conversation
rich-go's SetActivity could never report a dead pipe (it ignored both the write error and the IPC response and always returned nil), so v1.3.1 worked around it with a blind 3-minute reconnect timer. go-discordrpc does a read-after-write and returns real errors, so reconnection is now event-driven: on a failed SetActivity we drop the socket and redial on the next cycle. Removes lastLoginTime/connectionRefreshInterval and the timer-driven timestamp reset.
go-discordrpc is GPL-3.0; statically linking it into the release binaries requires the project to be GPL-compatible. Switch LICENSE to verbatim GPLv3.
📝 WalkthroughWalkthroughThe PR migrates Discord RPC integration from the ChangesDiscord RPC Library Migration from rich-go to go-discordrpc
Sequence Diagram(s)sequenceDiagram
participant keepStatus
participant rpcClient
participant Discord
participant App as App State
keepStatus->>rpcClient: SetActivity(basic payload)
alt Attempt 1 Success
rpcClient->>Discord: Update presence
rpcClient-->>keepStatus: success
else Attempt 1 Fails
keepStatus->>rpcClient: SetActivity(detailed payload with songLink)
alt Attempt 2 Success
rpcClient->>Discord: Update presence
rpcClient-->>keepStatus: success
else Both Fail
keepStatus->>keepStatus: log "will reconnect" warning
keepStatus->>rpcClient: Logout()
rpcClient->>Discord: Close connection
keepStatus->>App: reset ts to 0
Note over App: Next keepStatus cycle will re-login
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Pull request overview
This PR migrates Discord Rich Presence integration from hugolgst/rich-go to xeyossr/go-discordrpc, aiming to improve handling of dropped IPC connections and remove the previous periodic reconnect workaround. It also updates the project license to GPL-3.0 to align with the new dependency’s license.
Changes:
- Replaced
rich-goRPC usage with a sharedgo-discordrpcclient (rpcClient) and updated login/logout flow. - Updated presence update loop to drop/reconnect the RPC connection on
SetActivityfailures instead of using a periodic refresh timer. - Switched repository license from MIT to GPL-3.0 and updated Go module dependencies accordingly.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
rpc.go |
Introduces a persistent go-discordrpc client and updates login/logout behavior and logging. |
cycler.go |
Updates presence-setting calls to use rpcClient and adjusts reconnect-on-error logic. |
activity.go |
Switches the Activity/Button/Timestamps types import to go-discordrpc. |
go.mod |
Removes rich-go, adds go-discordrpc. |
go.sum |
Updates checksums to reflect the dependency switch. |
LICENSE |
Replaces MIT text with GPL-3.0 license text. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+23
to
+26
| if err := rpcClient.Login(); err != nil { | ||
| getRPCLogCtx().Warnln("Could not login to Discord.") | ||
| logout() | ||
| } else { | ||
| loggedIn = true | ||
| getRPCLogCtx().Debugln("Successfully logged into Discord's RPC Server.") | ||
| return |
Comment on lines
+33
to
38
| if err := rpcClient.Logout(); err != nil { | ||
| getRPCLogCtx().WithError(err).Debugln("Error closing Discord RPC connection.") | ||
| } | ||
| loggedIn = false | ||
| getRPCLogCtx().Debugln("Successfully logged out of Discord's RPC Server.") | ||
| } |
Comment on lines
26
to
+28
| if err != nil { | ||
| log.Warnln("Failed to keep activity.") | ||
| log.Warnln("Failed to keep activity. Dropping connection to reconnect next cycle.") | ||
| logout() |
Comment on lines
59
to
61
| if err1 != nil { | ||
| log.Info("Failed to set base RPC. Retrying with detailed payload.") | ||
| } else { |
Comment on lines
72
to
74
| } else { | ||
| log.Info("Failed to set detailed RPC.") | ||
| } |
Comment on lines
68
to
+71
| if err1 != nil { | ||
| log.Warnln("Both attempts to set RPC failed.") | ||
| log.Warnln("Both attempts to set RPC failed. Reconnecting next cycle.") | ||
| logout() | ||
| ts = time.Time{} |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
rpc.go (1)
32-38: 💤 Low valueConsider guarding
logout()with aloggedIncheck to avoid redundant operations.Currently
logout()unconditionally callsrpcClient.Logout()even if not logged in. While likely harmless if the library handles it gracefully, adding a guard similar tologin()would be more consistent and avoid unnecessary IPC calls.♻️ Optional: Add loggedIn guard to logout()
func logout() { + if !loggedIn { + return + } if err := rpcClient.Logout(); err != nil { getRPCLogCtx().WithError(err).Debugln("Error closing Discord RPC connection.") } loggedIn = false getRPCLogCtx().Debugln("Successfully logged out of Discord's RPC Server.") }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@rpc.go` around lines 32 - 38, Add a guard at the start of logout() to return early when loggedIn is false to avoid calling rpcClient.Logout() unnecessarily; specifically, check the package-level boolean loggedIn and if it's false simply log or return, otherwise proceed to call rpcClient.Logout(), set loggedIn = false and log via getRPCLogCtx(), keeping the existing error handling around rpcClient.Logout().
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@rpc.go`:
- Around line 32-38: Add a guard at the start of logout() to return early when
loggedIn is false to avoid calling rpcClient.Logout() unnecessarily;
specifically, check the package-level boolean loggedIn and if it's false simply
log or return, otherwise proceed to call rpcClient.Logout(), set loggedIn =
false and log via getRPCLogCtx(), keeping the existing error handling around
rpcClient.Logout().
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b9f9d3b5-eb00-42db-b1c2-d4c49d1e48f4
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
LICENSEactivity.gocycler.gogo.modrpc.go
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Switches Discord RPC to go-discordrpc for proper broken-pipe error handling (event-driven reconnect instead of the 3-minute timer hack); relicenses to GPL-3.0 since the new dependency is GPL.
Summary by CodeRabbit
Chores
Bug Fixes