-
Notifications
You must be signed in to change notification settings - Fork 44
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
connect() Socket API undefined #100
Comments
@elithrar Thank you for raising this issue! The minimal reproduction seems to be more simplified. So it seems not related to the original Issue. I'll separate a new Issue for that. Perhaps the original problem is caused by custom Dialer implementation. On the other hand, this problem is likely to be resolved by another issue (#90) that removes the dependency on Context. |
I don't know if it will be helpful, but here is an example of how I used go-sql-driver/mysql and connect(). |
Thanks for investigating! The full code (since it's a bit more verbose...) — as a work-in-progress / still very hacky — is below. With
Code: package main
import (
"context"
"database/sql"
"fmt"
"log"
"net"
"net/http"
"net/url"
"time"
"github.com/lib/pq"
"github.com/syumai/workers"
"github.com/syumai/workers/cloudflare"
"github.com/syumai/workers/cloudflare/sockets"
)
type HyperdriveDialer struct {
bindingName string
connectionString string
context context.Context
timeout time.Duration
}
func (hd *HyperdriveDialer) Dial(network, addr string) (net.Conn, error) {
return sockets.Connect(hd.context, hd.connectionString, &sockets.SocketOptions{SecureTransport: sockets.SecureTransportOff})
}
func (hd *HyperdriveDialer) DialTimeout(network string, addr string, timeout time.Duration) (net.Conn, error) {
// TODO(matt): Plumb through the timeout
return hd.Dial(network, addr)
}
func (hd *HyperdriveDialer) Address() string {
uri, err := url.Parse(hd.connectionString)
if err != nil {
return ""
}
return uri.Hostname()
}
func (hd *HyperdriveDialer) ConnectionString() string {
return hd.connectionString
}
func NewHyperdriveDialer(ctx context.Context, bindingName string) *HyperdriveDialer {
return &HyperdriveDialer{
connectionString: cloudflare.GetBinding(ctx, bindingName).Get("connectionString").String(),
context: ctx,
}
}
func main() {
http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
msg := "Hello!"
w.Write([]byte(msg))
})
http.HandleFunc("/query", queryDB)
workers.Serve(nil) // use http.DefaultServeMux
}
func queryDB(w http.ResponseWriter, r *http.Request) {
hd := NewHyperdriveDialer(r.Context(), "HYPERDRIVE")
connector, err := pq.NewConnector(hd.ConnectionString())
if err != nil {
log.Printf("#%v\n", err)
fmt.Fprintf(w, "bad")
return
}
log.Printf("connectionString: %#v\n", hd.ConnectionString())
connector.Dialer(hd)
db := sql.OpenDB(connector)
if err != nil {
log.Printf("opening db failed: %#v\n", err)
fmt.Fprintf(w, "bad")
return
}
if err := db.PingContext(r.Context()); err != nil {
log.Printf("failed to ping DB: %#v\n", err)
fmt.Fprintf(w, "failed to ping DB %#v\n", err)
return
}
rows, err := db.Query("SELECT * FROM pg_tables LIMIT 1")
if err != nil {
log.Printf("query failed: %#v\n", err)
fmt.Fprintf(w, "bad")
return
}
names := make([]string, 0)
for rows.Next() {
var name string
if err = rows.Scan(&name); err != nil {
break
}
names = append(names, name)
}
if err := rows.Close(); err != nil {
log.Printf("closing rows failed; %#v\n", err)
fmt.Fprintf(w, "bad")
return
}
fmt.Fprintf(w, "#%v\n", names)
} |
Dial(network, addr string) (net.Conn, error) { looks using correct Context, I wonder why this code is receiving By the way, #102 is merged and now |
Thank you @syumai! I've pulled the latest version of this package down — some progress but still running into an issue when attempting to use the socket. This could be outside of this lib: ➜ go get github.com/syumai/workers@01783f2
go: downloading github.com/syumai/workers v0.23.4-0.20240416160017-01783f2ba18c
go: upgraded github.com/syumai/workers v0.23.3 => v0.23.4-0.20240416160017-01783f2ba18c
(still investigating) |
I was also able to reproduce this issue, both deployed and locally. For context, this is connecting to posgres on Neon DB. Edit: I may have been overly ambitious with this reply - when I bypassed hyperdrive I used the default postgres driver which means we weren't making use of the custom dialer - dialing over sockets could still be an issue here |
I'll investigate this after #103 is resolved. |
This issue probably has the same problem as #103. |
Just updated to
Bypassing Hyperdrive and just going direct to the origin database fails with the same issue (just to remove Hyperdrive's proxy from the equation). A simple
|
OK, thank you for detail information! I'll continue investigation. |
Raising this as I debug in parallel.
Summary: It appears that the attempt to get a handler on the
connect
method fails and returns undefined: https://github.com/syumai/workers/blob/main/cloudflare/sockets/connect.go#L33 -> https://github.com/syumai/workers/blob/main/internal/runtimecontext/context.go#L34Dialer
implementation to havelib/pq
talk to Hyperdrive, andpq
would return the below and fail to actually establish a TCP connection:runtimecontext
I suspected I needed to plumb through acontext.Context
, but from the example below that doesn't seem to change anything here.Distilling this down into the minimal repro (below) I see this:
The text was updated successfully, but these errors were encountered: