Skip to content

Commit

Permalink
fix: handle proxy watcher errors (#149)
Browse files Browse the repository at this point in the history
I saw the todo so i tried my hand at handling proxy watcher errors. It
made me realize that the underlying mcnet library doesn't respect
context very well at all which is probably one of the most important
places to respect context actually. we should consider contributing
back to that library to upgrade net.Listen to use net.ListenConfig
which does respect context.
  • Loading branch information
george-e-shaw-iv authored May 18, 2024
1 parent e16a850 commit 273839b
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 25 deletions.
2 changes: 1 addition & 1 deletion cmd/minecraft-preempt/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func (c *Connection) Close() error {
//
// If the server is not running, it returns a status response with
// the server's status.
func (c *Connection) status(ctx context.Context, status cloud.ProviderStatus) error {
func (c *Connection) status(_ context.Context, status cloud.ProviderStatus) error {
if c.hooks.OnStatus != nil {
c.hooks.OnStatus()
}
Expand Down
74 changes: 52 additions & 22 deletions cmd/minecraft-preempt/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,40 +122,53 @@ func (p *Proxy) Start(ctx context.Context) error {
}
p.Listener = l

errChan := make(chan error)

// start the watcher
go func() {
if err := p.watcher(ctx); err != nil && !errors.Is(err, context.Canceled) {
p.log.Error("Failed to start watcher", "err", err)
// TODO(jaredallard): Trigger shutdown of proxy when this happens.
if err := p.watcher(ctx); err != nil {
p.log.Error("proxy server watcher encountered unrecoverable error", "err", err)

// signal to the main go-routine that the proxy has shut down.
errChan <- err
}
}()

connChan := make(chan *mcnet.Conn)
go func() {
for {
conn, err := p.accept()
if err != nil {
p.log.Error("failed to accept connection", "err", err)
} else {
connChan <- conn
}

if ctx.Err() != nil {
// We've probably already exited out of the main go-routine by now, but just incase we
// should communicate back.
errChan <- ctx.Err()
}
}
}()

p.log.Info("Proxy started", "address", p.listenAddress)
for {
if err := p.accept(ctx); err != nil {
p.log.Error("failed to accept connection", "err", err)
select {
case err := <-errChan:
return err
case conn := <-connChan:
if err := p.handleConnection(ctx, conn); err != nil {
p.log.Error("failed to handle connection", "err", err)
}
}
}
}

// accept accepts a connection on the proxy listener.
func (p *Proxy) accept(ctx context.Context) error {
rawConn, err := p.Listener.Accept()
if err != nil {
// handle context cancel or net closed
if errors.Is(err, context.Canceled) || errors.Is(err, net.ErrClosed) {
// context cancel should propagate the error
if errors.Is(err, context.Canceled) {
return err
}

// successful exit if we're closed
return nil
}

return fmt.Errorf("failed to accept connection: %w", err)
func (p *Proxy) handleConnection(ctx context.Context, rawConn *mcnet.Conn) error {
minecraftConn := &minecraft.Client{
Conn: rawConn,
}
minecraftConn := &minecraft.Client{Conn: &rawConn}

log := p.log.With("client", rawConn.Socket.RemoteAddr())
h, err := minecraftConn.Handshake()
Expand Down Expand Up @@ -215,6 +228,23 @@ func (p *Proxy) accept(ctx context.Context) error {
return nil
}

// accept accepts a connection on the proxy listener.
func (p *Proxy) accept() (*mcnet.Conn, error) {
// TODO(george-e-shaw-iv): Someone needs to go and make the underlying library respect context with net.ListenConfig.
rawConn, err := p.Listener.Accept()
if err != nil {
if errors.Is(err, context.Canceled) {
return nil, err // Context cancellation is a real error.
} else if errors.Is(err, net.ErrClosed) {
return nil, nil // Closed connections are fine.
}

// All other errors are unknown and bad.
return nil, fmt.Errorf("failed to accept connection: %w", err)
}
return &rawConn, nil
}

// Stop stops the server
func (p *Proxy) Stop(ctx context.Context) error {
if p.Listener != nil {
Expand Down
3 changes: 1 addition & 2 deletions internal/cloud/gcp/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,10 @@ import (
"fmt"
"strings"

"github.com/jaredallard/minecraft-preempt/v3/internal/cloud"

compute "cloud.google.com/go/compute/apiv1"
"cloud.google.com/go/compute/apiv1/computepb"
"cloud.google.com/go/compute/metadata"
"github.com/jaredallard/minecraft-preempt/v3/internal/cloud"
)

var (
Expand Down

0 comments on commit 273839b

Please sign in to comment.