From 0919fb2da6df2941df7a6172aa8cebac5985b7dc Mon Sep 17 00:00:00 2001 From: Dane Everitt Date: Wed, 3 Mar 2021 20:56:18 -0800 Subject: [PATCH] Improve error handling and reporting for server installation & process boot --- environment/docker.go | 3 ++- environment/docker/environment.go | 7 ++++--- installer/installer.go | 18 ++++++++++-------- router/router_system.go | 5 ++++- server/manager.go | 4 ++-- server/server.go | 10 +++++----- server/update.go | 20 +++++++++++--------- 7 files changed, 38 insertions(+), 29 deletions(-) diff --git a/environment/docker.go b/environment/docker.go index f47e370..e3c4bf4 100644 --- a/environment/docker.go +++ b/environment/docker.go @@ -5,6 +5,7 @@ import ( "strconv" "sync" + "emperror.dev/errors" "github.com/apex/log" "github.com/docker/docker/api/types" "github.com/docker/docker/api/types/network" @@ -23,7 +24,7 @@ func Docker() (*client.Client, error) { _conce.Do(func() { _client, err = client.NewClientWithOpts(client.FromEnv, client.WithAPIVersionNegotiation()) }) - return _client, err + return _client, errors.Wrap(err, "environment/docker: could not create client") } // ConfigureDocker configures the required network for the docker environment. diff --git a/environment/docker/environment.go b/environment/docker/environment.go index 99282b0..1265172 100644 --- a/environment/docker/environment.go +++ b/environment/docker/environment.go @@ -54,9 +54,10 @@ type Environment struct { st *system.AtomicString } -// Creates a new base Docker environment. The ID passed through will be the ID that is used to -// reference the container from here on out. This should be unique per-server (we use the UUID -// by default). The container does not need to exist at this point. +// New creates a new base Docker environment. The ID passed through will be the +// ID that is used to reference the container from here on out. This should be +// unique per-server (we use the UUID by default). The container does not need +// to exist at this point. func New(id string, m *Metadata, c *environment.Configuration) (*Environment, error) { cli, err := environment.Docker() if err != nil { diff --git a/installer/installer.go b/installer/installer.go index 495f27c..d2b9bd3 100644 --- a/installer/installer.go +++ b/installer/installer.go @@ -45,21 +45,21 @@ func New(ctx context.Context, manager *server.Manager, data []byte) (*Installer, // Unmarshal the environment variables from the request into the server struct. if b, _, _, err := jsonparser.Get(data, "environment"); err != nil { - return nil, err + return nil, errors.WithStackIf(err) } else { cfg.EnvVars = make(environment.Variables) if err := json.Unmarshal(b, &cfg.EnvVars); err != nil { - return nil, err + return nil, errors.WrapIf(err, "installer: could not unmarshal environment variables for server") } } // Unmarshal the allocation mappings from the request into the server struct. if b, _, _, err := jsonparser.Get(data, "allocations", "mappings"); err != nil { - return nil, err + return nil, errors.WithStackIf(err) } else { cfg.Allocations.Mappings = make(map[string][]int) if err := json.Unmarshal(b, &cfg.Allocations.Mappings); err != nil { - return nil, err + return nil, errors.Wrap(err, "installer: could not unmarshal allocation mappings") } } @@ -68,16 +68,18 @@ func New(ctx context.Context, manager *server.Manager, data []byte) (*Installer, c, err := manager.Client().GetServerConfiguration(ctx, cfg.Uuid) if err != nil { if !remote.IsRequestError(err) { - return nil, err + return nil, errors.WithStackIf(err) } - return nil, errors.New(err.Error()) + return nil, errors.WrapIf(err, "installer: could not get server configuration from remote API") } // Create a new server instance using the configuration we wrote to the disk // so that everything gets instantiated correctly on the struct. s, err := manager.InitServer(c) - - return &Installer{server: s}, err + if err != nil { + return nil, errors.WrapIf(err, "installer: could not init server instance") + } + return &Installer{server: s}, nil } // Uuid returns the UUID associated with this installer instance. diff --git a/router/router_system.go b/router/router_system.go index b9b0460..8869398 100644 --- a/router/router_system.go +++ b/router/router_system.go @@ -2,6 +2,7 @@ package router import ( "bytes" + "fmt" "net/http" "strings" @@ -38,6 +39,8 @@ func postCreateServer(c *gin.Context) { buf := bytes.Buffer{} buf.ReadFrom(c.Request.Body) + fmt.Println(buf.String()) + install, err := installer.New(c.Request.Context(), manager, buf.Bytes()) if err != nil { if installer.IsValidationError(err) { @@ -47,7 +50,7 @@ func postCreateServer(c *gin.Context) { return } - NewTrackedError(err).Abort(c) + // middleware.CaptureAndAbort(c, err) return } diff --git a/server/manager.go b/server/manager.go index 6f8d4fa..49bf2c5 100644 --- a/server/manager.go +++ b/server/manager.go @@ -169,7 +169,7 @@ func (m *Manager) ReadStates() (map[string]string, error) { func (m *Manager) InitServer(data remote.ServerConfigurationResponse) (*Server, error) { s, err := New(m.client) if err != nil { - return nil, errors.WithMessage(err, "loader: failed to instantiate empty server struct") + return nil, err } if err := s.UpdateDataStructure(data.Settings); err != nil { return nil, err @@ -222,7 +222,7 @@ func (m *Manager) init(ctx context.Context) error { if !remote.IsRequestError(err) { return errors.WithStackIf(err) } - return errors.New(err.Error()) + return errors.WrapIf(err, "manager: failed to retrieve server configurations") } start := time.Now() diff --git a/server/server.go b/server/server.go index 9efe1e5..4618eac 100644 --- a/server/server.go +++ b/server/server.go @@ -70,8 +70,8 @@ type Server struct { wsBagLocker sync.Mutex } -// Returns a new server instance with a context and all of the default values set on -// the instance. +// New returns a new server instance with a context and all of the default +// values set on the struct. func New(client remote.Client) (*Server, error) { ctx, cancel := context.WithCancel(context.Background()) s := Server{ @@ -82,16 +82,16 @@ func New(client remote.Client) (*Server, error) { transferring: system.NewAtomicBool(false), } if err := defaults.Set(&s); err != nil { - return nil, err + return nil, errors.Wrap(err, "server: could not set default values for struct") } if err := defaults.Set(&s.cfg); err != nil { - return nil, err + return nil, errors.Wrap(err, "server: could not set defaults for server configuration") } s.resources.State = system.NewAtomicString(environment.ProcessOfflineState) return &s, nil } -// Returns the UUID for the server instance. +// Id returns the UUID for the server instance. func (s *Server) Id() string { return s.Config().GetUuid() } diff --git a/server/update.go b/server/update.go index c952979..c65fdc4 100644 --- a/server/update.go +++ b/server/update.go @@ -2,6 +2,7 @@ package server import ( "encoding/json" + "fmt" "emperror.dev/errors" "github.com/buger/jsonparser" @@ -9,24 +10,25 @@ import ( "github.com/pterodactyl/wings/environment" ) -// Merges data passed through in JSON form into the existing server object. -// Any changes to the build settings will apply immediately in the environment -// if the environment supports it. +// UpdateDataStructure merges data passed through in JSON form into the existing +// server object. Any changes to the build settings will apply immediately in +// the environment if the environment supports it. // // The server will be marked as requiring a rebuild on the next boot sequence, // it is up to the specific environment to determine what needs to happen when // that is the case. func (s *Server) UpdateDataStructure(data []byte) error { src := new(Configuration) + fmt.Println("got data", string(data)) if err := json.Unmarshal(data, src); err != nil { - return err + return errors.Wrap(err, "server/update: could not unmarshal source data into Configuration struct") } // Don't allow obviously corrupted data to pass through into this function. If the UUID // doesn't match something has gone wrong and the API is attempting to meld this server // instance into a totally different one, which would be bad. if src.Uuid != "" && s.Id() != "" && src.Uuid != s.Id() { - return errors.New("attempting to merge a data stack with an invalid UUID") + return errors.New("server/update: attempting to merge a data stack with an invalid UUID") } // Grab a copy of the configuration to work on. @@ -48,7 +50,7 @@ func (s *Server) UpdateDataStructure(data []byte) error { // Merge the new data object that we have received with the existing server data object // and then save it to the disk so it is persistent. if err := mergo.Merge(&c, src, mergo.WithOverride); err != nil { - return err + return errors.WithStack(err) } // Don't explode if we're setting CPU limits to 0. Mergo sees that as an empty value @@ -62,7 +64,7 @@ func (s *Server) UpdateDataStructure(data []byte) error { // request is going to be boolean. Allegedly. if v, err := jsonparser.GetBoolean(data, "container", "oom_disabled"); err != nil { if err != jsonparser.KeyPathNotFoundError { - return err + return errors.WithStack(err) } } else { c.Build.OOMDisabled = v @@ -71,7 +73,7 @@ func (s *Server) UpdateDataStructure(data []byte) error { // Mergo also cannot handle this boolean value. if v, err := jsonparser.GetBoolean(data, "suspended"); err != nil { if err != jsonparser.KeyPathNotFoundError { - return err + return errors.WithStack(err) } } else { c.Suspended = v @@ -79,7 +81,7 @@ func (s *Server) UpdateDataStructure(data []byte) error { if v, err := jsonparser.GetBoolean(data, "skip_egg_scripts"); err != nil { if err != jsonparser.KeyPathNotFoundError { - return err + return errors.WithStack(err) } } else { c.SkipEggScripts = v