Skip to content
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

feat(daemon): add ErrorResponse to allow external error creation #303

Merged
merged 7 commits into from
Sep 29, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 0 additions & 21 deletions client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -403,26 +403,6 @@ type response struct {
Maintenance *Error `json:"maintenance"`
}

// Error is the real value of response.Result when an error occurs.
type Error struct {
Kind string `json:"kind"`
Value interface{} `json:"value"`
Message string `json:"message"`

StatusCode int
flotter marked this conversation as resolved.
Show resolved Hide resolved
}

func (e *Error) Error() string {
return e.Message
}

const (
ErrorKindLoginRequired = "login-required"
ErrorKindSystemRestart = "system-restart"
ErrorKindDaemonRestart = "daemon-restart"
ErrorKindNoDefaultServices = "no-default-services"
)

func (rsp *response) err(cli *Client) error {
if cli != nil {
maintErr := rsp.Maintenance
Expand All @@ -441,7 +421,6 @@ func (rsp *response) err(cli *Client) error {
if err != nil || resultErr.Message == "" {
return fmt.Errorf("server error: %q", rsp.Status)
}
resultErr.StatusCode = rsp.StatusCode

return &resultErr
}
Expand Down
43 changes: 43 additions & 0 deletions client/errors.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// Copyright (c) 2023 Canonical Ltd
flotter marked this conversation as resolved.
Show resolved Hide resolved
//
// This program is free software: you can redistribute it and/or modify
// it under the terms of the GNU General Public License version 3 as
// published by the Free Software Foundation.
//
// This program is distributed in the hope that it will be useful,
// but WITHOUT ANY WARRANTY; without even the implied warranty of
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
// GNU General Public License for more details.
//
// You should have received a copy of the GNU General Public License
// along with this program. If not, see <http://www.gnu.org/licenses/>.

package client

// Error is the real value of response.Result when an error type response occurs.
type Error struct {
Message string `json:"message"`
Kind ErrorKind `json:"kind,omitempty"`
flotter marked this conversation as resolved.
Show resolved Hide resolved
Value interface{} `json:"value,omitempty"`
flotter marked this conversation as resolved.
Show resolved Hide resolved
}

func (e *Error) Error() string {
return e.Message
}

type ErrorKind string

// Error kinds for use as a response result.
const (
ErrorKindLoginRequired = ErrorKind("login-required")
flotter marked this conversation as resolved.
Show resolved Hide resolved
ErrorKindNoDefaultServices = ErrorKind("no-default-services")
flotter marked this conversation as resolved.
Show resolved Hide resolved
ErrorKindNotFound = ErrorKind("not-found")
ErrorKindPermissionDenied = ErrorKind("permission-denied")
ErrorKindGenericFileError = ErrorKind("generic-file-error")
flotter marked this conversation as resolved.
Show resolved Hide resolved
)

// Maintenance error kinds for use only inside the maintenance field of responses.
const (
ErrorKindSystemRestart = ErrorKind("system-restart")
ErrorKindDaemonRestart = ErrorKind("daemon-restart")
)
4 changes: 2 additions & 2 deletions client/files_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ func (cs *clientSuite) TestMakeDirFailsOnDirectory(c *C) {
clientErr, ok := err.(*client.Error)
c.Assert(ok, Equals, true)
c.Assert(clientErr.Message, Equals, "could not bar")
c.Assert(clientErr.Kind, Equals, "permission-denied")
c.Assert(clientErr.Kind, Equals, client.ErrorKindPermissionDenied)
flotter marked this conversation as resolved.
Show resolved Hide resolved

c.Assert(cs.req.URL.Path, Equals, "/v1/files")
c.Assert(cs.req.Method, Equals, "POST")
Expand Down Expand Up @@ -517,7 +517,7 @@ func (cs *clientSuite) TestRemovePathFailsOnPath(c *C) {
clientErr, ok := err.(*client.Error)
c.Assert(ok, Equals, true)
c.Assert(clientErr.Message, Equals, "could not bar")
c.Assert(clientErr.Kind, Equals, "permission-denied")
c.Assert(clientErr.Kind, Equals, client.ErrorKindPermissionDenied)

c.Assert(cs.req.URL.Path, Equals, "/v1/files")
c.Assert(cs.req.Method, Equals, "POST")
Expand Down
2 changes: 1 addition & 1 deletion internals/cli/cmd_mkdir_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,7 @@ func (s *PebbleSuite) TestMkdirFailsOnDirectory(c *C) {
clientErr, ok := err.(*client.Error)
c.Assert(ok, Equals, true)
c.Assert(clientErr.Message, Equals, "could not bar")
c.Assert(clientErr.Kind, Equals, "permission-denied")
c.Assert(clientErr.Kind, Equals, client.ErrorKindPermissionDenied)

c.Assert(rest, HasLen, 1)
c.Check(s.Stdout(), Equals, "")
Expand Down
2 changes: 1 addition & 1 deletion internals/cli/cmd_rm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ func (s *PebbleSuite) TestRmFailsOnPath(c *C) {
clientErr, ok := err.(*client.Error)
c.Assert(ok, Equals, true)
c.Assert(clientErr.Message, Equals, "could not baz")
c.Assert(clientErr.Kind, Equals, "permission-denied")
c.Assert(clientErr.Kind, Equals, client.ErrorKindPermissionDenied)
c.Check(s.Stdout(), Equals, "")
c.Check(s.Stderr(), Equals, "")

Expand Down
9 changes: 5 additions & 4 deletions internals/daemon/api_changes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (

"gopkg.in/check.v1"

"github.com/canonical/pebble/client"
flotter marked this conversation as resolved.
Show resolved Hide resolved
"github.com/canonical/pebble/internals/overlord/state"
)

Expand Down Expand Up @@ -384,7 +385,7 @@ func (s *apiSuite) TestWaitChangeInvalidTimeout(c *check.C) {
c.Check(rec.Code, check.Equals, 400)
c.Check(rsp.Status, check.Equals, 400)
c.Check(rsp.Type, check.Equals, ResponseTypeError)
result := rsp.Result.(*errorResult)
result := rsp.Result.(*client.Error)
c.Check(result.Message, check.Matches, "invalid timeout .*")
}

Expand Down Expand Up @@ -419,7 +420,7 @@ func (s *apiSuite) TestWaitChangeCancel(c *check.C) {
c.Check(rec.Code, check.Equals, 500)
c.Check(rsp.Status, check.Equals, 500)
c.Check(rsp.Type, check.Equals, ResponseTypeError)
result := rsp.Result.(*errorResult)
result := rsp.Result.(*client.Error)
c.Check(result.Message, check.Equals, "request cancelled")
}

Expand All @@ -428,7 +429,7 @@ func (s *apiSuite) TestWaitChangeTimeout(c *check.C) {
c.Check(rec.Code, check.Equals, 504)
c.Check(rsp.Status, check.Equals, 504)
c.Check(rsp.Type, check.Equals, ResponseTypeError)
result := rsp.Result.(*errorResult)
result := rsp.Result.(*client.Error)
c.Check(result.Message, check.Matches, "timed out waiting for change .*")
}

Expand All @@ -440,7 +441,7 @@ func (s *apiSuite) TestWaitChangeTimeoutCancel(c *check.C) {
c.Check(rec.Code, check.Equals, 500)
c.Check(rsp.Status, check.Equals, 500)
c.Check(rsp.Type, check.Equals, ResponseTypeError)
result := rsp.Result.(*errorResult)
result := rsp.Result.(*client.Error)
c.Check(result.Message, check.Equals, "request cancelled")
}

Expand Down
17 changes: 9 additions & 8 deletions internals/daemon/api_files.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import (
"syscall"
"time"

"github.com/canonical/pebble/client"
flotter marked this conversation as resolved.
Show resolved Hide resolved
"github.com/canonical/pebble/internals/osutil"
"github.com/canonical/pebble/internals/osutil/sys"
)
Expand Down Expand Up @@ -67,8 +68,8 @@ func v1GetFiles(_ *Command, req *http.Request, _ *UserState) Response {
}

type fileResult struct {
Path string `json:"path"`
Error *errorResult `json:"error,omitempty"`
Path string `json:"path"`
Error *client.Error `json:"error,omitempty"`
}

// Reading files
Expand Down Expand Up @@ -160,20 +161,20 @@ func readFile(path string, mw *multipart.Writer) error {
return nil
}

func fileErrorToResult(err error) *errorResult {
func fileErrorToResult(err error) *client.Error {
if err == nil {
return nil
}
var kind errorKind
var kind client.ErrorKind
switch {
case errors.Is(err, os.ErrPermission):
kind = errorKindPermissionDenied
kind = client.ErrorKindPermissionDenied
case errors.Is(err, os.ErrNotExist):
kind = errorKindNotFound
kind = client.ErrorKindNotFound
default:
kind = errorKindGenericFileError
kind = client.ErrorKindGenericFileError
}
return &errorResult{
return &client.Error{
Kind: kind,
Message: err.Error(),
}
Expand Down
8 changes: 5 additions & 3 deletions internals/daemon/api_plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (

. "gopkg.in/check.v1"
"gopkg.in/yaml.v3"

"github.com/canonical/pebble/client"
)

var planLayer = `
Expand Down Expand Up @@ -54,7 +56,7 @@ func (s *apiSuite) TestGetPlanErrors(c *C) {
c.Assert(rec.Code, Equals, test.status)
c.Assert(rsp.Status, Equals, test.status)
c.Assert(rsp.Type, Equals, ResponseTypeError)
c.Assert(rsp.Result.(*errorResult).Message, Matches, test.message)
c.Assert(rsp.Result.(*client.Error).Message, Matches, test.message)
}
}

Expand Down Expand Up @@ -123,7 +125,7 @@ func (s *apiSuite) TestLayersErrors(c *C) {
c.Assert(rec.Code, Equals, test.status)
c.Assert(rsp.Status, Equals, test.status)
c.Assert(rsp.Type, Equals, ResponseTypeError)
c.Assert(rsp.Result.(*errorResult).Message, Matches, test.message)
c.Assert(rsp.Result.(*client.Error).Message, Matches, test.message)
}
}

Expand Down Expand Up @@ -195,6 +197,6 @@ func (s *apiSuite) TestLayersCombineFormatError(c *C) {
c.Assert(rec.Code, Equals, http.StatusBadRequest)
c.Assert(rsp.Status, Equals, http.StatusBadRequest)
c.Assert(rsp.Type, Equals, ResponseTypeError)
result := rsp.Result.(*errorResult)
result := rsp.Result.(*client.Error)
c.Assert(result.Message, Matches, `layer "base" must define "override" for service "dynamic"`)
}
3 changes: 2 additions & 1 deletion internals/daemon/api_services.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (

"github.com/canonical/x-go/strutil"

"github.com/canonical/pebble/client"
"github.com/canonical/pebble/internals/overlord/servstate"
"github.com/canonical/pebble/internals/overlord/state"
)
Expand Down Expand Up @@ -88,7 +89,7 @@ func v1PostServices(c *Command, r *http.Request, _ *UserState) Response {
if len(services) == 0 {
return SyncResponse(&resp{
Type: ResponseTypeError,
Result: &errorResult{Kind: errorKindNoDefaultServices, Message: "no default services"},
Result: &client.Error{Kind: client.ErrorKindNoDefaultServices, Message: "no default services"},
Status: 400,
})
}
Expand Down
9 changes: 5 additions & 4 deletions internals/daemon/api_signals_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (

. "gopkg.in/check.v1"

"github.com/canonical/pebble/client"
"github.com/canonical/pebble/internals/overlord/servstate"
)

Expand Down Expand Up @@ -66,7 +67,7 @@ services:
rec = httptest.NewRecorder()
rsp.ServeHTTP(rec, req)
c.Check(rec.Result().StatusCode, Equals, 500)
errResult, ok := rsp.Result.(*errorResult)
errResult, ok := rsp.Result.(*client.Error)
c.Assert(ok, Equals, true)
c.Assert(errResult.Message, Matches, `cannot send signal to "test1": invalid signal name "FOOBAR"`)

Expand Down Expand Up @@ -102,7 +103,7 @@ func (s *apiSuite) TestSignalsBadBody(c *C) {
rec := httptest.NewRecorder()
rsp.ServeHTTP(rec, req)
c.Check(rec.Result().StatusCode, Equals, 400)
errResult, ok := rsp.Result.(*errorResult)
errResult, ok := rsp.Result.(*client.Error)
c.Assert(ok, Equals, true)
c.Assert(errResult.Message, Matches, "cannot decode request body: .*")
}
Expand All @@ -115,7 +116,7 @@ func (s *apiSuite) TestSignalsNoServices(c *C) {
rec := httptest.NewRecorder()
rsp.ServeHTTP(rec, req)
c.Check(rec.Result().StatusCode, Equals, 400)
errResult, ok := rsp.Result.(*errorResult)
errResult, ok := rsp.Result.(*client.Error)
c.Assert(ok, Equals, true)
c.Assert(errResult.Message, Equals, "must specify one or more services")
}
Expand All @@ -129,7 +130,7 @@ func (s *apiSuite) TestSignalsServiceNotRunning(c *C) {
rec := httptest.NewRecorder()
rsp.ServeHTTP(rec, req)
c.Check(rec.Result().StatusCode, Equals, 500)
errResult, ok := rsp.Result.(*errorResult)
errResult, ok := rsp.Result.(*client.Error)
c.Assert(ok, Equals, true)
c.Assert(errResult.Message, Matches, `cannot send signal to "test1": service is not running`)
}
7 changes: 4 additions & 3 deletions internals/daemon/daemon.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (
"github.com/gorilla/mux"
"gopkg.in/tomb.v2"

"github.com/canonical/pebble/client"
"github.com/canonical/pebble/internals/logger"
"github.com/canonical/pebble/internals/osutil"
"github.com/canonical/pebble/internals/osutil/sys"
Expand Down Expand Up @@ -280,11 +281,11 @@ func (c *Command) ServeHTTP(w http.ResponseWriter, r *http.Request) {
st.Unlock()
switch rst {
case restart.RestartSystem:
rsp.transmitMaintenance(errorKindSystemRestart, "system is restarting")
rsp.transmitMaintenance(client.ErrorKindSystemRestart, "system is restarting")
case restart.RestartDaemon:
rsp.transmitMaintenance(errorKindDaemonRestart, "daemon is restarting")
rsp.transmitMaintenance(client.ErrorKindDaemonRestart, "daemon is restarting")
case restart.RestartSocket:
rsp.transmitMaintenance(errorKindDaemonRestart, "daemon is stopping to wait for socket activation")
rsp.transmitMaintenance(client.ErrorKindDaemonRestart, "daemon is stopping to wait for socket activation")
}
if rsp.Type != ResponseTypeError {
st.Lock()
Expand Down
13 changes: 7 additions & 6 deletions internals/daemon/daemon_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"github.com/gorilla/mux"
. "gopkg.in/check.v1"

"github.com/canonical/pebble/client"
"github.com/canonical/pebble/internals/logger"
"github.com/canonical/pebble/internals/osutil"
"github.com/canonical/pebble/internals/overlord"
Expand Down Expand Up @@ -264,7 +265,7 @@ func (s *daemonSuite) TestCommandRestartingState(c *C) {
cmd.ServeHTTP(rec, req)
c.Check(rec.Code, Equals, 200)
var rst struct {
Maintenance *errorResult `json:"maintenance"`
Maintenance *client.Error `json:"maintenance"`
}
err = json.Unmarshal(rec.Body.Bytes(), &rst)
c.Assert(err, IsNil)
Expand All @@ -280,8 +281,8 @@ func (s *daemonSuite) TestCommandRestartingState(c *C) {
c.Check(rec.Code, Equals, 200)
err = json.Unmarshal(rec.Body.Bytes(), &rst)
c.Assert(err, IsNil)
c.Check(rst.Maintenance, DeepEquals, &errorResult{
Kind: errorKindSystemRestart,
c.Check(rst.Maintenance, DeepEquals, &client.Error{
Kind: client.ErrorKindSystemRestart,
Message: "system is restarting",
})

Expand All @@ -293,8 +294,8 @@ func (s *daemonSuite) TestCommandRestartingState(c *C) {
c.Check(rec.Code, Equals, 200)
err = json.Unmarshal(rec.Body.Bytes(), &rst)
c.Assert(err, IsNil)
c.Check(rst.Maintenance, DeepEquals, &errorResult{
Kind: errorKindDaemonRestart,
c.Check(rst.Maintenance, DeepEquals, &client.Error{
Kind: client.ErrorKindDaemonRestart,
Message: "daemon is restarting",
})
}
Expand Down Expand Up @@ -1138,7 +1139,7 @@ func (s *daemonSuite) TestDegradedModeReply(c *C) {
rec = doTestReq(c, cmd, "POST")
c.Check(rec.Code, Equals, 500)
// verify we get the error
var v struct{ Result errorResult }
var v struct{ Result client.Error }
c.Assert(json.NewDecoder(rec.Body).Decode(&v), IsNil)
c.Check(v.Result.Message, Equals, "foo error")

Expand Down
Loading