chore: fix nuked nhooyr/websocket dependency #1
Conversation
WalkthroughThe changes update the Go module version and migrate the WebSocket dependency from Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant grpcwebproxy
participant websocket (github.com/coder/websocket)
participant Backend
Client->>grpcwebproxy: Initiate WebSocket connection
grpcwebproxy->>websocket (github.com/coder/websocket): Handle WebSocket handshake and communication
grpcwebproxy->>Backend: Forward gRPC request
Backend-->>grpcwebproxy: Respond with gRPC data
grpcwebproxy->>websocket (github.com/coder/websocket): Send data over WebSocket
grpcwebproxy-->>Client: Return response via WebSocket
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
go/grpcweb/websocket_wrapper.go (1)
15-15: Validate websocket wrapper functionality remains intact.
After migrating to"github.com/coder/websocket", ensure that critical methods used bywebSocketResponseWriterandwebSocketWrappedReader(Write,Read,Ping,SetReadLimit,Close, etc.) behave identically to the previous implementation. Consider adding or updating unit/integration tests for websocket flows.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (5)
go.mod(2 hunks)go/grpcweb/options.go(1 hunks)go/grpcweb/websocket_wrapper.go(1 hunks)go/grpcweb/wrapper.go(1 hunks)go/grpcwebproxy/main.go(1 hunks)
🔇 Additional comments (4)
go/grpcweb/wrapper.go (1)
14-14:❓ Verification inconclusive
Verify websocket API compatibility.
The import of"github.com/coder/websocket"replacesnhooyr.io/websocket. Please confirm that the new module exports all required functions and types (e.g.,Accept,AcceptOptions,MessageBinary,Conn.SetReadLimit,Conn.Ping) used inHandleGrpcWebsocketRequestand related code.Run:
🏁 Script executed:
rg -R "AcceptOptions" -n $(go env GOPATH)/pkg/mod/github.com/coder/[email protected] rg -R "func Accept" -n $(go env GOPATH)/pkg/mod/github.com/coder/[email protected]Length of output: 1015
🏁 Script executed:
#!/bin/bash # Verify that github.com/coder/[email protected] exports the needed symbols MODDIR="$(go env GOPATH)/pkg/mod/github.com/coder/[email protected]" rg "AcceptOptions" -n "$MODDIR" rg -e "func Accept" -n "$MODDIR" rg "MessageBinary" -n "$MODDIR" rg "SetReadLimit" -n "$MODDIR" rg "Ping" -n "$MODDIR"Length of output: 999
🏁 Script executed:
#!/bin/bash # Determine the exact version of github.com/coder/websocket in use rg "github.com/coder/websocket" -n go.mod go.sumLength of output: 286
🏁 Script executed:
#!/bin/bash # Locate the module directory via go list and verify exported symbols MODDIR=$(go list -m -json github.com/coder/[email protected] | awk -F '"' '/"Dir":/ {print $4}') echo "Module directory: $MODDIR" rg "AcceptOptions" -n "$MODDIR" rg -e "func Accept" -n "$MODDIR" rg "MessageBinary" -n "$MODDIR" rg "SetReadLimit" -n "$MODDIR" rg "Ping" -n "$MODDIR"Length of output: 470
Manual Verification Required: Confirm websocket API Compatibility
Unable to locate the downloadedgithub.com/coder/[email protected]source in this environment for automated inspection. Please ensure that the new module provides the same exports used in your code:
- Accept(ctx, w, r, opts…) / AcceptOptions
- MessageBinary
- Conn.SetReadLimit
- Conn.Ping
These are referenced in
go/grpcweb/wrapper.go’sHandleGrpcWebsocketRequest.go/grpcweb/options.go (1)
10-10: Dependency update approved.
The import of"github.com/coder/websocket"aligns with the updatedgo.modand matches usage ofwebsocket.CompressionMode. No issues found.go.mod (2)
3-3: Verify Go version bump compatibility.
Thegodirective has been updated to1.23. Ensure that all CI pipelines and contributor environments support this version. Update documentation (e.g., README) to reflect the new minimum Go version if necessary.
7-7: Confirm new websocket dependency version.
Therequireblock now includesgithub.com/coder/websocket v1.8.13. Please verify that this version is correct, trusted, and free of known vulnerabilities. Consider runninggo mod tidy,go mod verify, and checking relevant advisory databases.
| @@ -10,7 +10,7 @@ import ( | |||
| "sync" | |||
| "time" | |||
|
|
|||
| "nhooyr.io/websocket" | |||
| "github.com/coder/websocket" | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure import grouping and remove old websocket import.
The new import "github.com/coder/websocket" is correctly added but it's currently splitting standard library imports (e.g., it sits before "crypto/tls"). Consider moving it to the external dependencies block (after all stdlib imports) to abide by import grouping conventions. Also verify that all occurrences of the old "nhooyr.io/websocket" import have been removed across the codebase.
Run:
🏁 Script executed:
rg -n "nhooyr.io/websocket"Length of output: 205
🏁 Script executed:
#!/bin/bash
sed -n '1,50p' go/grpcwebproxy/main.goLength of output: 2599
Import grouping: move github.com/coder/websocket into the external imports block
Verified that there are no remaining code references to nhooyr.io/websocket (only a mention in CHANGELOG.md). Please adjust the import order so that all stdlib imports—including "crypto/tls"—are grouped together, followed by third-party imports:
go/grpcwebproxy/main.go:
import (
"fmt"
"log"
"net"
"net/http"
_ "net/http/pprof" // register in DefaultServerMux
"os"
"sync"
"time"
- "github.com/coder/websocket"
"crypto/tls"
+ "github.com/coder/websocket"
grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware"
grpc_logrus "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus"
grpc_prometheus "github.com/grpc-ecosystem/go-grpc-prometheus"
…
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "github.com/coder/websocket" | |
| import ( | |
| "fmt" | |
| "log" | |
| "net" | |
| "net/http" | |
| _ "net/http/pprof" // register in DefaultServerMux | |
| "os" | |
| "sync" | |
| "time" | |
| "crypto/tls" | |
| "github.com/coder/websocket" | |
| grpc_middleware "github.com/grpc-ecosystem/go-grpc-middleware" | |
| grpc_logrus "github.com/grpc-ecosystem/go-grpc-middleware/logging/logrus" | |
| grpc_prometheus "github.com/grpc-ecosystem/go-grpc-prometheus" | |
| … | |
| ) |
Summary by CodeRabbit