From 5245c8eefdd0b275539a817ae62f4b330d17e5b1 Mon Sep 17 00:00:00 2001 From: Marcos Navarro Date: Mon, 30 Sep 2024 15:49:52 -0600 Subject: [PATCH 1/3] Handle preflight errors on content: No permissions, not found, unreachable --- internal/clients/connect/client.go | 1 + internal/clients/connect/client_connect.go | 5 + .../clients/connect/client_connect_test.go | 22 ++++ internal/clients/connect/mock_client.go | 5 + internal/clients/http_client/http_client.go | 9 ++ .../clients/http_client/http_client_test.go | 44 +++++++ internal/events/cli_emitter.go | 2 +- internal/events/events.go | 14 +-- internal/publish/publish.go | 36 +++++- internal/publish/publish_test.go | 119 +++++++++++++----- internal/types/error.go | 13 +- 11 files changed, 219 insertions(+), 51 deletions(-) create mode 100644 internal/clients/http_client/http_client_test.go diff --git a/internal/clients/connect/client.go b/internal/clients/connect/client.go index eec8e3ea7..94da17797 100644 --- a/internal/clients/connect/client.go +++ b/internal/clients/connect/client.go @@ -22,6 +22,7 @@ type User struct { type APIClient interface { TestAuthentication(logging.Logger) (*User, error) + ContentDetails(contentID types.ContentID, body *ConnectContent, log logging.Logger) error CreateDeployment(*ConnectContent, logging.Logger) (types.ContentID, error) UpdateDeployment(types.ContentID, *ConnectContent, logging.Logger) error SetEnvVars(types.ContentID, config.Environment, logging.Logger) error diff --git a/internal/clients/connect/client_connect.go b/internal/clients/connect/client_connect.go index aa34ec919..6c95dfbaf 100644 --- a/internal/clients/connect/client_connect.go +++ b/internal/clients/connect/client_connect.go @@ -179,6 +179,11 @@ type connectGetContentDTO struct { // Owner *ownerOutputDTO `json:"owner,omitempty"` } +func (c *ConnectClient) ContentDetails(contentID types.ContentID, body *ConnectContent, log logging.Logger) error { + url := fmt.Sprintf("/__api__/v1/content/%s", contentID) + return c.client.Get(url, body, log) +} + func (c *ConnectClient) CreateDeployment(body *ConnectContent, log logging.Logger) (types.ContentID, error) { content := connectGetContentDTO{} err := c.client.Post("/__api__/v1/content", body, &content, log) diff --git a/internal/clients/connect/client_connect_test.go b/internal/clients/connect/client_connect_test.go index 481f78969..c27b30fef 100644 --- a/internal/clients/connect/client_connect_test.go +++ b/internal/clients/connect/client_connect_test.go @@ -568,3 +568,25 @@ func (s *ConnectClientSuite) TestTestAuthenticationNotPublisher() { s.NotNil(err) s.ErrorContains(err, "user account bob with role 'viewer' does not have permission to publish content") } + +func (s *ConnectClientSuite) TestContentDetails() { + lgr := logging.New() + content := &ConnectContent{} + httpClient := &http_client.MockHTTPClient{} + httpClient.On("Get", "/__api__/v1/content/e8922765-4880-43cd-abc0-d59fe59b8b4b", content, lgr).Return(nil) + + client := &ConnectClient{ + client: httpClient, + } + + err := client.ContentDetails("e8922765-4880-43cd-abc0-d59fe59b8b4b", content, lgr) + s.NoError(err) + httpClient.AssertExpectations(s.T()) + + expectedErr := errors.New("unreachable") + httpClient.On("Get", "/__api__/v1/content/cf3d3afe-2076-4812-825a-28237252030b", content, lgr).Return(expectedErr) + + err = client.ContentDetails("cf3d3afe-2076-4812-825a-28237252030b", content, lgr) + s.ErrorIs(err, expectedErr) + httpClient.AssertExpectations(s.T()) +} diff --git a/internal/clients/connect/mock_client.go b/internal/clients/connect/mock_client.go index 5577ad120..94b028666 100644 --- a/internal/clients/connect/mock_client.go +++ b/internal/clients/connect/mock_client.go @@ -37,6 +37,11 @@ func (m *MockClient) TestAuthentication(log logging.Logger) (*User, error) { } } +func (m *MockClient) ContentDetails(id types.ContentID, s *ConnectContent, log logging.Logger) error { + args := m.Called(id, s, log) + return args.Error(0) +} + func (m *MockClient) CreateDeployment(s *ConnectContent, log logging.Logger) (types.ContentID, error) { args := m.Called(s, log) return args.Get(0).(types.ContentID), args.Error(1) diff --git a/internal/clients/http_client/http_client.go b/internal/clients/http_client/http_client.go index 84e72936e..ca2de9a85 100644 --- a/internal/clients/http_client/http_client.go +++ b/internal/clients/http_client/http_client.go @@ -241,3 +241,12 @@ func NewHTTPClientForAccount(account *accounts.Account, timeout time.Duration, l Transport: authTransport, }, nil } + +func IsHTTPAgentErrorStatusOf(err error, status int) (*types.AgentError, bool) { + if aerr, isAgentErr := err.(*types.AgentError); isAgentErr { + if httperr, isHttpErr := aerr.Err.(*HTTPError); isHttpErr { + return aerr, httperr.Status == status + } + } + return nil, false +} diff --git a/internal/clients/http_client/http_client_test.go b/internal/clients/http_client/http_client_test.go new file mode 100644 index 000000000..244a2d812 --- /dev/null +++ b/internal/clients/http_client/http_client_test.go @@ -0,0 +1,44 @@ +package http_client + +import ( + "errors" + "net/http" + "testing" + + "github.com/posit-dev/publisher/internal/types" + "github.com/posit-dev/publisher/internal/util/utiltest" + "github.com/stretchr/testify/suite" +) + +// Copyright (C) 2024 by Posit Software, PBC. + +type HttpClientSuite struct { + utiltest.Suite +} + +func TestHttpClientSuite(t *testing.T) { + suite.Run(t, new(HttpClientSuite)) +} + +func (s *HttpClientSuite) TestIsHTTPAgentErrorStatusOf() { + agentErr := types.NewAgentError( + types.ErrorDeploymentTargetNotFound, + NewHTTPError("", "", http.StatusNotFound), + nil, + ) + + // With a true agent error + resultingErr, yesItIs := IsHTTPAgentErrorStatusOf(agentErr, http.StatusNotFound) + s.Equal(yesItIs, true) + s.Equal(agentErr, resultingErr) + + // With a true agent error, but a status that it is not + resultingErr, yesItIs = IsHTTPAgentErrorStatusOf(agentErr, http.StatusBadGateway) + s.Equal(yesItIs, false) + s.Equal(agentErr, resultingErr) + + // With a non agent error + resultingErr, yesItIs = IsHTTPAgentErrorStatusOf(errors.New("nope"), http.StatusNotFound) + s.Equal(yesItIs, false) + s.Nil(resultingErr) +} diff --git a/internal/events/cli_emitter.go b/internal/events/cli_emitter.go index 077e603b3..15976f91f 100644 --- a/internal/events/cli_emitter.go +++ b/internal/events/cli_emitter.go @@ -91,7 +91,7 @@ func (e *cliEmitter) Emit(event *Event) error { fmt.Fprintln(e.writer, "[OK]", formatEventData(event.Data)) case FailurePhase: e.writer.NeedNewline = false - fmt.Fprintln(e.writer, "[ERROR]", event.errCode, formatEventData(event.Data)) + fmt.Fprintln(e.writer, "[ERROR]", event.ErrCode, formatEventData(event.Data)) } return nil } diff --git a/internal/events/events.go b/internal/events/events.go index 74aa2a612..6a7e003cd 100644 --- a/internal/events/events.go +++ b/internal/events/events.go @@ -17,13 +17,13 @@ type EventData = types.ErrorData var NoData = struct{}{} type Event struct { - Time time.Time - Type EventType - Data EventData + Time time.Time + Type EventType + Data EventData + ErrCode ErrorCode - op Operation - phase Phase - errCode ErrorCode + op Operation + phase Phase } // We use Operation and Phase to construct the event Type. @@ -72,9 +72,9 @@ func New(op Operation, phase Phase, errCode ErrorCode, data any) *Event { Time: time.Now(), Type: EventTypeOf(op, phase), Data: eventData, + ErrCode: errCode, op: op, phase: phase, - errCode: errCode, } } diff --git a/internal/publish/publish.go b/internal/publish/publish.go index b2cf8a9e1..a7927f9c5 100644 --- a/internal/publish/publish.go +++ b/internal/publish/publish.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "maps" + "net/http" "os" "time" @@ -13,6 +14,7 @@ import ( "github.com/posit-dev/publisher/internal/accounts" "github.com/posit-dev/publisher/internal/bundles" "github.com/posit-dev/publisher/internal/clients/connect" + "github.com/posit-dev/publisher/internal/clients/http_client" "github.com/posit-dev/publisher/internal/config" "github.com/posit-dev/publisher/internal/deployment" "github.com/posit-dev/publisher/internal/events" @@ -201,11 +203,11 @@ func (p *defaultPublisher) PublishDirectory() error { func (p *defaultPublisher) writeDeploymentRecord() error { if p.SaveName == "" { // Redeployment - p.log.Debug("No SaveName found in deployment. Redeploying.", "deployment", p.TargetName) + p.log.Debug("No SaveName found while redeploying.", "deployment", p.TargetName) p.SaveName = p.TargetName } else { // Initial deployment - p.log.Debug("SaveName found in deployment. First deployment.", "deployment", p.SaveName) + p.log.Debug("SaveName found in first deployment.", "deployment", p.SaveName) p.TargetName = p.SaveName } @@ -265,6 +267,28 @@ func (p *defaultPublisher) createDeploymentRecord( return p.writeDeploymentRecord() } +// Does the target exist and the user has permissions to it? +func (p *defaultPublisher) targetPreflightChecks(client connect.APIClient, contentID types.ContentID) error { + p.log.Debug("Running deployment preflight checks", "content_id", contentID) + content := &connect.ConnectContent{} + err := client.ContentDetails(contentID, content, p.log) + if err != nil { + if aerr, isForbidden := http_client.IsHTTPAgentErrorStatusOf(err, http.StatusForbidden); isForbidden { + p.log.Debug("Insufficient permissions. Content deployment cannot proceed", "content_id", contentID) + aerr.Code = types.ErrorDeploymentTargetIsForbidden + return aerr + } + if aerr, isNotFound := http_client.IsHTTPAgentErrorStatusOf(err, http.StatusNotFound); isNotFound { + p.log.Debug("Content cannot be found. Deployment cannot proceed", "content_id", contentID) + aerr.Code = types.ErrorDeploymentTargetNotFound + return aerr + } + p.log.Debug("Deployment target cannot be reached. Halting deployment", "content_id", contentID) + return types.NewAgentError(types.ErrorDeploymentTargetUnreachable, err, nil) + } + return nil +} + func (p *defaultPublisher) publishWithClient( account *accounts.Account, client connect.APIClient) error { @@ -293,13 +317,15 @@ func (p *defaultPublisher) publishWithClient( if p.isDeployed() { contentID = p.Target.ID p.log.Info("Updating deployment", "content_id", contentID) + err = p.targetPreflightChecks(client, contentID) } else { // Create a new deployment; we will update it with details later. contentID, err = p.createDeployment(client) - if err != nil { - return err - } } + if err != nil { + return err + } + err = p.createDeploymentRecord(contentID, account) if err != nil { return types.OperationError(events.PublishCreateNewDeploymentOp, err) diff --git a/internal/publish/publish_test.go b/internal/publish/publish_test.go index 91f88c573..964bdf707 100644 --- a/internal/publish/publish_test.go +++ b/internal/publish/publish_test.go @@ -5,6 +5,7 @@ import ( "bytes" "errors" "log/slog" + "net/http" "strings" "testing" "time" @@ -12,6 +13,7 @@ import ( "github.com/posit-dev/publisher/internal/accounts" "github.com/posit-dev/publisher/internal/bundles" "github.com/posit-dev/publisher/internal/clients/connect" + "github.com/posit-dev/publisher/internal/clients/http_client" "github.com/posit-dev/publisher/internal/config" "github.com/posit-dev/publisher/internal/deployment" "github.com/posit-dev/publisher/internal/events" @@ -50,6 +52,19 @@ func (m *mockPackageMapper) GetManifestPackages(base util.AbsolutePath, lockfile } } +type publishErrsMock struct { + rPackageErr error + authErr error + capErr error + checksErr error + createErr error + envVarErr error + uploadErr error + deployErr error + waitErr error + validateErr error +} + func TestPublishSuite(t *testing.T) { suite.Run(t, new(PublishSuite)) } @@ -108,7 +123,7 @@ func (s *PublishSuite) TestNewFromState() { } func (s *PublishSuite) TestPublishWithClientNewSuccess() { - s.publishWithClient(nil, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) + s.publishWithClient(nil, &publishErrsMock{}, nil) } func (s *PublishSuite) TestPublishWithClientNewUpdate() { @@ -116,111 +131,144 @@ func (s *PublishSuite) TestPublishWithClientNewUpdate() { target.ID = "myContentID" // Make CreatedAt earlier so it will differ from DeployedAt. target.CreatedAt = time.Now().Add(-time.Hour).Format(time.RFC3339) - s.publishWithClient(target, nil, nil, nil, nil, nil, nil, nil, nil, nil, nil) + s.publishWithClient(target, &publishErrsMock{}, nil) } func (s *PublishSuite) TestPublishWithClientNewFailAuth() { authErr := errors.New("error from TestAuthentication") - s.publishWithClient(nil, nil, authErr, nil, nil, nil, nil, nil, nil, nil, authErr) + s.publishWithClient(nil, &publishErrsMock{authErr: authErr}, authErr) } func (s *PublishSuite) TestPublishWithClientNewFailCapabilities() { capErr := errors.New("error from CheckCapabilities") - s.publishWithClient(nil, nil, nil, capErr, nil, nil, nil, nil, nil, nil, capErr) + s.publishWithClient(nil, &publishErrsMock{capErr: capErr}, capErr) } func (s *PublishSuite) TestPublishWithClientNewFailCreate() { createErr := errors.New("error from Create") - s.publishWithClient(nil, nil, nil, nil, createErr, nil, nil, nil, nil, nil, createErr) + s.publishWithClient(nil, &publishErrsMock{createErr: createErr}, createErr) } func (s *PublishSuite) TestPublishWithClientNewFailEnvVars() { envVarErr := errors.New("error from SetEnvVars") - s.publishWithClient(nil, nil, nil, nil, nil, envVarErr, nil, nil, nil, nil, envVarErr) + s.publishWithClient(nil, &publishErrsMock{envVarErr: envVarErr}, envVarErr) } func (s *PublishSuite) TestPublishWithClientNewFailUpload() { uploadErr := errors.New("error from Upload") - s.publishWithClient(nil, nil, nil, nil, nil, nil, uploadErr, nil, nil, nil, uploadErr) + s.publishWithClient(nil, &publishErrsMock{uploadErr: uploadErr}, uploadErr) } func (s *PublishSuite) TestPublishWithClientNewFailDeploy() { deployErr := errors.New("error from Deploy") - s.publishWithClient(nil, nil, nil, nil, nil, nil, nil, deployErr, nil, nil, deployErr) + s.publishWithClient(nil, &publishErrsMock{deployErr: deployErr}, deployErr) } func (s *PublishSuite) TestPublishWithClientNewFailWaitForTask() { waitErr := errors.New("error from WaitForTask") - s.publishWithClient(nil, nil, nil, nil, nil, nil, nil, nil, waitErr, nil, waitErr) + s.publishWithClient(nil, &publishErrsMock{waitErr: waitErr}, waitErr) } func (s *PublishSuite) TestPublishWithClientNewFailValidation() { validateErr := errors.New("error from ValidateDeployment") - s.publishWithClient(nil, nil, nil, nil, nil, nil, nil, nil, nil, validateErr, validateErr) + s.publishWithClient(nil, &publishErrsMock{validateErr: validateErr}, validateErr) } func (s *PublishSuite) TestPublishWithClientNewRPackages() { rPackageErr := errors.New("error from GetManifestPackages") - s.publishWithClient(nil, rPackageErr, nil, nil, nil, nil, nil, nil, nil, nil, rPackageErr) + s.publishWithClient(nil, &publishErrsMock{rPackageErr: rPackageErr}, rPackageErr) } func (s *PublishSuite) TestPublishWithClientRedeployFailAuth() { target := deployment.New() target.ID = "myContentID" authErr := errors.New("error from TestAuthentication") - s.publishWithClient(target, nil, authErr, nil, nil, nil, nil, nil, nil, nil, authErr) + s.publishWithClient(target, &publishErrsMock{authErr: authErr}, authErr) } func (s *PublishSuite) TestPublishWithClientRedeployFailCapabilities() { target := deployment.New() target.ID = "myContentID" capErr := errors.New("error from CheckCapabilities") - s.publishWithClient(target, nil, nil, capErr, nil, nil, nil, nil, nil, nil, capErr) + s.publishWithClient(target, &publishErrsMock{capErr: capErr}, capErr) +} + +func (s *PublishSuite) TestPublishWithClientRedeployNoPermissionsErr() { + target := deployment.New() + target.ID = "myContentID" + checksErr := types.NewAgentError( + types.ErrorDeploymentTargetIsForbidden, + http_client.NewHTTPError("", "", http.StatusForbidden), + nil, + ) + s.publishWithClient(target, &publishErrsMock{checksErr: checksErr}, checksErr) +} + +func (s *PublishSuite) TestPublishWithClientRedeployContentNotFound() { + target := deployment.New() + target.ID = "myContentID" + checksErr := types.NewAgentError( + types.ErrorDeploymentTargetNotFound, + http_client.NewHTTPError("", "", http.StatusNotFound), + nil, + ) + s.publishWithClient(target, &publishErrsMock{checksErr: checksErr}, checksErr) +} + +func (s *PublishSuite) TestPublishWithClientRedeployCannotReachContentTarget() { + target := deployment.New() + target.ID = "myContentID" + checksErr := types.NewAgentError( + types.ErrorDeploymentTargetUnreachable, + http_client.NewHTTPError("", "", http.StatusServiceUnavailable), + nil, + ) + s.publishWithClient(target, &publishErrsMock{checksErr: checksErr}, checksErr) } func (s *PublishSuite) TestPublishWithClientRedeployFailUpdate() { target := deployment.New() target.ID = "myContentID" updateErr := errors.New("error from Update") - s.publishWithClient(target, nil, nil, nil, updateErr, nil, nil, nil, nil, nil, updateErr) + s.publishWithClient(target, &publishErrsMock{createErr: updateErr}, updateErr) } func (s *PublishSuite) TestPublishWithClientRedeployFailEnvVars() { target := deployment.New() target.ID = "myContentID" envVarErr := errors.New("error from SetEnvVars") - s.publishWithClient(target, nil, nil, nil, nil, envVarErr, nil, nil, nil, nil, envVarErr) + s.publishWithClient(target, &publishErrsMock{envVarErr: envVarErr}, envVarErr) } func (s *PublishSuite) TestPublishWithClientRedeployFailUpload() { target := deployment.New() target.ID = "myContentID" uploadErr := errors.New("error from Upload") - s.publishWithClient(target, nil, nil, nil, nil, nil, uploadErr, nil, nil, nil, uploadErr) + s.publishWithClient(target, &publishErrsMock{uploadErr: uploadErr}, uploadErr) } func (s *PublishSuite) TestPublishWithClientRedeployFailDeploy() { target := deployment.New() target.ID = "myContentID" deployErr := errors.New("error from Deploy") - s.publishWithClient(target, nil, nil, nil, nil, nil, nil, deployErr, nil, nil, deployErr) + s.publishWithClient(target, &publishErrsMock{deployErr: deployErr}, deployErr) } func (s *PublishSuite) TestPublishWithClientRedeployFailWaitForTask() { target := deployment.New() target.ID = "myContentID" waitErr := errors.New("error from WaitForTask") - s.publishWithClient(target, nil, nil, nil, nil, nil, nil, nil, waitErr, nil, waitErr) + s.publishWithClient(target, &publishErrsMock{waitErr: waitErr}, waitErr) } func (s *PublishSuite) TestPublishWithClientRedeployFailValidation() { validateErr := errors.New("error from ValidateDeployment") - s.publishWithClient(nil, nil, nil, nil, nil, nil, nil, nil, nil, validateErr, validateErr) + s.publishWithClient(nil, &publishErrsMock{validateErr: validateErr}, validateErr) } func (s *PublishSuite) publishWithClient( target *deployment.Deployment, - rPackageErr, authErr, capErr, createErr, envVarErr, uploadErr, deployErr, waitErr, validateErr, + errsMock *publishErrsMock, expectedErr error) { account := &accounts.Account{ @@ -235,16 +283,17 @@ func (s *PublishSuite) publishWithClient( client := connect.NewMockClient() if target == nil { - client.On("CreateDeployment", mock.Anything, mock.Anything).Return(myContentID, createErr) + client.On("CreateDeployment", mock.Anything, mock.Anything).Return(myContentID, errsMock.createErr) } - client.On("TestAuthentication", mock.Anything).Return(&connect.User{}, authErr) - client.On("CheckCapabilities", mock.Anything, mock.Anything, mock.Anything).Return(capErr) - client.On("UpdateDeployment", myContentID, mock.Anything, mock.Anything).Return(createErr) - client.On("SetEnvVars", myContentID, mock.Anything, mock.Anything).Return(envVarErr) - client.On("UploadBundle", myContentID, mock.Anything, mock.Anything).Return(myBundleID, uploadErr) - client.On("DeployBundle", myContentID, myBundleID, mock.Anything).Return(myTaskID, deployErr) - client.On("WaitForTask", myTaskID, mock.Anything, mock.Anything).Return(waitErr) - client.On("ValidateDeployment", myContentID, mock.Anything).Return(validateErr) + client.On("TestAuthentication", mock.Anything).Return(&connect.User{}, errsMock.authErr) + client.On("CheckCapabilities", mock.Anything, mock.Anything, mock.Anything).Return(errsMock.capErr) + client.On("ContentDetails", myContentID, mock.Anything, mock.Anything).Return(errsMock.checksErr) + client.On("UpdateDeployment", myContentID, mock.Anything, mock.Anything).Return(errsMock.createErr) + client.On("SetEnvVars", myContentID, mock.Anything, mock.Anything).Return(errsMock.envVarErr) + client.On("UploadBundle", myContentID, mock.Anything, mock.Anything).Return(myBundleID, errsMock.uploadErr) + client.On("DeployBundle", myContentID, myBundleID, mock.Anything).Return(myTaskID, errsMock.deployErr) + client.On("WaitForTask", myTaskID, mock.Anything, mock.Anything).Return(errsMock.waitErr) + client.On("ValidateDeployment", myContentID, mock.Anything).Return(errsMock.validateErr) cfg := config.New() cfg.Type = config.ContentTypePythonDash @@ -296,8 +345,8 @@ func (s *PublishSuite) publishWithClient( } rPackageMapper := &mockPackageMapper{} - if rPackageErr != nil { - rPackageMapper.On("GetManifestPackages", mock.Anything, mock.Anything, mock.Anything).Return(nil, rPackageErr) + if errsMock.rPackageErr != nil { + rPackageMapper.On("GetManifestPackages", mock.Anything, mock.Anything, mock.Anything).Return(nil, errsMock.rPackageErr) } else { rPackageMapper.On("GetManifestPackages", mock.Anything, mock.Anything, mock.Anything).Return(bundles.PackageMap{}, nil) } @@ -320,7 +369,11 @@ func (s *PublishSuite) publishWithClient( s.NotEqual(stateStore.Target.CreatedAt, stateStore.Target.DeployedAt) } } - couldCreateDeployment := (rPackageErr == nil && authErr == nil && capErr == nil && createErr == nil) + couldCreateDeployment := (errsMock.rPackageErr == nil && + errsMock.authErr == nil && + errsMock.capErr == nil && + errsMock.checksErr == nil && + errsMock.createErr == nil) if (stateStore.Target != nil) || couldCreateDeployment { // Either a pre-existing deployment record, or we got far enough to create one recordPath := deployment.GetDeploymentPath(stateStore.Dir, recordName) @@ -341,7 +394,7 @@ func (s *PublishSuite) publishWithClient( s.Equal("https://connect.example.com/connect/#/apps/myContentID/logs", record.LogsURL) // Files are written after upload. - if uploadErr == nil { + if errsMock.uploadErr == nil { s.Contains(record.Files, "app.py") s.Contains(record.Files, "requirements.txt") s.Equal([]string{"flask"}, record.Requirements) diff --git a/internal/types/error.go b/internal/types/error.go index 1e1e8ba17..2b2c3b6ac 100644 --- a/internal/types/error.go +++ b/internal/types/error.go @@ -11,11 +11,14 @@ type ErrorData map[string]any type Operation string const ( - ErrorResourceNotFound ErrorCode = "resourceNotFound" - ErrorInvalidTOML ErrorCode = "invalidTOML" - ErrorUnknownTOMLKey ErrorCode = "unknownTOMLKey" - ErrorInvalidConfigFiles ErrorCode = "invalidConfigFiles" - ErrorUnknown ErrorCode = "unknown" + ErrorResourceNotFound ErrorCode = "resourceNotFound" + ErrorInvalidTOML ErrorCode = "invalidTOML" + ErrorUnknownTOMLKey ErrorCode = "unknownTOMLKey" + ErrorInvalidConfigFiles ErrorCode = "invalidConfigFiles" + ErrorDeploymentTargetNotFound ErrorCode = "deploymentTargetNotFound" + ErrorDeploymentTargetIsForbidden ErrorCode = "deploymentTargetIsForbidden" + ErrorDeploymentTargetUnreachable ErrorCode = "deploymentTargetUnreachable" + ErrorUnknown ErrorCode = "unknown" ) type EventableError interface { From 5f4ea046e10df92895195f3b7060dfd8db5ac4f2 Mon Sep 17 00:00:00 2001 From: Marcos Navarro Date: Tue, 1 Oct 2024 10:47:29 -0600 Subject: [PATCH 2/3] Handle publishing event errors on extension UI --- extensions/vscode/src/api/types/events.ts | 7 +- extensions/vscode/src/eventErrors.test.ts | 95 +++++++++++++++++++++++ extensions/vscode/src/eventErrors.ts | 46 +++++++++++ extensions/vscode/src/utils/errorTypes.ts | 7 +- extensions/vscode/src/views/logs.ts | 20 +++-- 5 files changed, 165 insertions(+), 10 deletions(-) create mode 100644 extensions/vscode/src/eventErrors.test.ts create mode 100644 extensions/vscode/src/eventErrors.ts diff --git a/extensions/vscode/src/api/types/events.ts b/extensions/vscode/src/api/types/events.ts index 362df881b..8654c35e4 100644 --- a/extensions/vscode/src/api/types/events.ts +++ b/extensions/vscode/src/api/types/events.ts @@ -1,5 +1,7 @@ // Copyright (C) 2023 by Posit Software, PBC. +import { ErrorCode } from "../../utils/errorTypes"; + export enum EventSourceReadyState { CONNECTING = 0, OPEN = 1, @@ -298,10 +300,11 @@ export function getLocalId(arg: EventStreamMessage) { return arg.data.localId; } -export interface EventStreamMessage { +export interface EventStreamMessage> { type: EventSubscriptionTarget; time: string; - data: Record; + data: T; + errCode?: ErrorCode; error?: string; } diff --git a/extensions/vscode/src/eventErrors.test.ts b/extensions/vscode/src/eventErrors.test.ts new file mode 100644 index 000000000..72a1e26f8 --- /dev/null +++ b/extensions/vscode/src/eventErrors.test.ts @@ -0,0 +1,95 @@ +// Copyright (C) 2024 by Posit Software, PBC. + +import { describe, expect, test } from "vitest"; +import { EventStreamMessage } from "./api"; +import { + EventStreamMessageErrorCoded, + isCodedEventErrorMessage, + isEvtErrTargetNotFound, + isEvtErrTargetForbidden, + handleEventCodedError, +} from "./eventErrors"; +import { ErrorCode } from "./utils/errorTypes"; + +const mkEventStreamMsg = ( + data = {}, + errCode?: ErrorCode, +): EventStreamMessage => { + return { + type: "publish/failure", + time: "Tue Oct 01 2024 10:00:00 GMT-0600", + data, + errCode, + error: "failed to publish", + }; +}; + +describe("Event errors", () => { + test("isCodedEventErrorMessage", () => { + // Message without error code + let streamMsg = mkEventStreamMsg(); + let result = isCodedEventErrorMessage(streamMsg); + expect(result).toBe(false); + + // Message with error code + streamMsg = mkEventStreamMsg({}, "deploymentTargetNotFound"); + result = isCodedEventErrorMessage(streamMsg); + expect(result).toBe(true); + }); + + test("isEvtErrTargetNotFound", () => { + // Message with another error code + let streamMsg = mkEventStreamMsg({}, "unknown"); + let result = isEvtErrTargetNotFound( + streamMsg as EventStreamMessageErrorCoded, + ); + expect(result).toBe(false); + + // Message with error code + streamMsg = mkEventStreamMsg({}, "deploymentTargetNotFound"); + result = isEvtErrTargetNotFound(streamMsg as EventStreamMessageErrorCoded); + expect(result).toBe(true); + }); + + test("isEvtErrTargetForbidden", () => { + // Message with another error code + let streamMsg = mkEventStreamMsg({}, "unknown"); + let result = isEvtErrTargetForbidden( + streamMsg as EventStreamMessageErrorCoded, + ); + expect(result).toBe(false); + + // Message with error code + streamMsg = mkEventStreamMsg({}, "deploymentTargetIsForbidden"); + result = isEvtErrTargetForbidden(streamMsg as EventStreamMessageErrorCoded); + expect(result).toBe(true); + }); + + test("handleEventCodedError", () => { + const msgData = { + dashboardUrl: "https://here.it.is/content/abcdefg", + error: "A possum on the fridge", + }; + let streamMsg = mkEventStreamMsg(msgData, "unknown"); + let resultMsg = handleEventCodedError( + streamMsg as EventStreamMessageErrorCoded, + ); + expect(resultMsg).toBe("Unknown error: A possum on the fridge"); + + streamMsg = mkEventStreamMsg(msgData, "deploymentTargetNotFound"); + resultMsg = handleEventCodedError( + streamMsg as EventStreamMessageErrorCoded, + ); + expect(resultMsg).toBe( + `Content at https://here.it.is/content/abcdefg could not be found. Please, verify the content "id" is accurate.`, + ); + + streamMsg = mkEventStreamMsg(msgData, "deploymentTargetIsForbidden"); + resultMsg = handleEventCodedError( + streamMsg as EventStreamMessageErrorCoded, + ); + expect(resultMsg).toBe( + "You don't have enough permissions to deploy to https://here.it.is/content/abcdefg. Please, verify the credentials in use.", + ); + }); +}); diff --git a/extensions/vscode/src/eventErrors.ts b/extensions/vscode/src/eventErrors.ts new file mode 100644 index 000000000..0f68a7c40 --- /dev/null +++ b/extensions/vscode/src/eventErrors.ts @@ -0,0 +1,46 @@ +// Copyright (C) 2024 by Posit Software, PBC. + +import { EventStreamMessage } from "./api/types/events"; +import { ErrorCode } from "./utils/errorTypes"; + +export interface EventStreamMessageErrorCoded> + extends EventStreamMessage { + errCode: ErrorCode; +} + +export function isCodedEventErrorMessage( + msg: EventStreamMessage, +): msg is EventStreamMessageErrorCoded { + return msg.errCode !== undefined; +} + +type deploymentEvtErr = { + dashboardUrl: string; + localId: string; + error: string; +}; + +export const isEvtErrTargetNotFound = ( + emsg: EventStreamMessageErrorCoded, +): emsg is EventStreamMessageErrorCoded => { + return emsg.errCode === "deploymentTargetNotFound"; +}; + +export const isEvtErrTargetForbidden = ( + emsg: EventStreamMessageErrorCoded, +): emsg is EventStreamMessageErrorCoded => { + return emsg.errCode === "deploymentTargetIsForbidden"; +}; + +export const handleEventCodedError = ( + emsg: EventStreamMessageErrorCoded, +): string => { + if (isEvtErrTargetNotFound(emsg)) { + return `Content at ${emsg.data.dashboardUrl} could not be found. Please, verify the content "id" is accurate.`; + } + if (isEvtErrTargetForbidden(emsg)) { + return `You don't have enough permissions to deploy to ${emsg.data.dashboardUrl}. Please, verify the credentials in use.`; + } + const unknownErrMsg = emsg.data.error || emsg.data.message; + return `Unknown error: ${unknownErrMsg}`; +}; diff --git a/extensions/vscode/src/utils/errorTypes.ts b/extensions/vscode/src/utils/errorTypes.ts index df3e9f5fd..fd813d4e7 100644 --- a/extensions/vscode/src/utils/errorTypes.ts +++ b/extensions/vscode/src/utils/errorTypes.ts @@ -2,12 +2,15 @@ import { AxiosError, AxiosResponse, isAxiosError } from "axios"; -type ErrorCode = +export type ErrorCode = | "unknown" | "resourceNotFound" | "invalidTOML" | "unknownTOMLKey" - | "invalidConfigFile"; + | "invalidConfigFile" + | "deploymentTargetNotFound" + | "deploymentTargetIsForbidden" + | "deploymentTargetUnreachable"; export type axiosErrorWithJson = AxiosError & { diff --git a/extensions/vscode/src/views/logs.ts b/extensions/vscode/src/views/logs.ts index cf6a8f352..633c0d3fc 100644 --- a/extensions/vscode/src/views/logs.ts +++ b/extensions/vscode/src/views/logs.ts @@ -16,6 +16,10 @@ import { } from "vscode"; import { EventStream, displayEventStreamMessage } from "src/events"; +import { + isCodedEventErrorMessage, + handleEventCodedError, +} from "src/eventErrors"; import { EventStreamMessage, @@ -195,12 +199,16 @@ export class LogsTreeDataProvider implements TreeDataProvider { if (enhancedError && enhancedError.buttonStr) { options.push(enhancedError.buttonStr); } - const selection = await window.showErrorMessage( - msg.data.cancelled === "true" - ? `Deployment cancelled: ${msg.data.message}` - : `Deployment failed: ${msg.data.message}`, - ...options, - ); + let errorMessage = ""; + if (isCodedEventErrorMessage(msg)) { + errorMessage = handleEventCodedError(msg); + } else { + errorMessage = + msg.data.cancelled === "true" + ? `Deployment cancelled: ${msg.data.message}` + : `Deployment failed: ${msg.data.message}`; + } + const selection = await window.showErrorMessage(errorMessage, ...options); if (selection === showLogsOption) { await commands.executeCommand(Commands.Logs.Focus); } else if (selection === enhancedError?.buttonStr) { From 221f6e0cd6774ea2b01f15ee063d2e2f3290bfae Mon Sep 17 00:00:00 2001 From: Marcos Navarro Date: Thu, 3 Oct 2024 15:53:00 -0600 Subject: [PATCH 3/3] PR feedback: Less error codes --- extensions/vscode/src/eventErrors.test.ts | 71 +++++++------------ extensions/vscode/src/eventErrors.ts | 19 ++--- extensions/vscode/src/utils/errorTypes.ts | 4 +- internal/clients/connect/content.go | 12 ++++ internal/clients/connect/mock_client.go | 5 ++ .../clients/http_client/http_client_test.go | 3 +- internal/publish/publish.go | 45 ++++++++---- internal/publish/publish_test.go | 62 +++++++++++----- internal/types/error.go | 13 +++- internal/types/error_test.go | 17 +++++ 10 files changed, 155 insertions(+), 96 deletions(-) diff --git a/extensions/vscode/src/eventErrors.test.ts b/extensions/vscode/src/eventErrors.test.ts index 72a1e26f8..d46d5991d 100644 --- a/extensions/vscode/src/eventErrors.test.ts +++ b/extensions/vscode/src/eventErrors.test.ts @@ -5,91 +5,68 @@ import { EventStreamMessage } from "./api"; import { EventStreamMessageErrorCoded, isCodedEventErrorMessage, - isEvtErrTargetNotFound, - isEvtErrTargetForbidden, + isEvtErrDeploymentFailed, handleEventCodedError, } from "./eventErrors"; import { ErrorCode } from "./utils/errorTypes"; -const mkEventStreamMsg = ( - data = {}, - errCode?: ErrorCode, -): EventStreamMessage => { - return { +function mkEventStreamMsg(data: Record): EventStreamMessage; +function mkEventStreamMsg( + data: Record, + errCode: ErrorCode, +): EventStreamMessageErrorCoded; +function mkEventStreamMsg(data: Record, errCode?: ErrorCode) { + const smsg: EventStreamMessage = { type: "publish/failure", time: "Tue Oct 01 2024 10:00:00 GMT-0600", data, - errCode, error: "failed to publish", }; -}; + if (errCode) { + smsg.errCode = errCode; + } + return smsg; +} describe("Event errors", () => { test("isCodedEventErrorMessage", () => { // Message without error code - let streamMsg = mkEventStreamMsg(); + let streamMsg = mkEventStreamMsg({}); let result = isCodedEventErrorMessage(streamMsg); expect(result).toBe(false); // Message with error code - streamMsg = mkEventStreamMsg({}, "deploymentTargetNotFound"); + streamMsg = mkEventStreamMsg({}, "deployFailed"); result = isCodedEventErrorMessage(streamMsg); expect(result).toBe(true); }); - test("isEvtErrTargetNotFound", () => { + test("isEvtErrDeploymentFailed", () => { // Message with another error code let streamMsg = mkEventStreamMsg({}, "unknown"); - let result = isEvtErrTargetNotFound( - streamMsg as EventStreamMessageErrorCoded, - ); - expect(result).toBe(false); - - // Message with error code - streamMsg = mkEventStreamMsg({}, "deploymentTargetNotFound"); - result = isEvtErrTargetNotFound(streamMsg as EventStreamMessageErrorCoded); - expect(result).toBe(true); - }); - - test("isEvtErrTargetForbidden", () => { - // Message with another error code - let streamMsg = mkEventStreamMsg({}, "unknown"); - let result = isEvtErrTargetForbidden( - streamMsg as EventStreamMessageErrorCoded, - ); + let result = isEvtErrDeploymentFailed(streamMsg); expect(result).toBe(false); // Message with error code - streamMsg = mkEventStreamMsg({}, "deploymentTargetIsForbidden"); - result = isEvtErrTargetForbidden(streamMsg as EventStreamMessageErrorCoded); + streamMsg = mkEventStreamMsg({}, "deployFailed"); + result = isEvtErrDeploymentFailed(streamMsg); expect(result).toBe(true); }); test("handleEventCodedError", () => { const msgData = { dashboardUrl: "https://here.it.is/content/abcdefg", + message: "Deployment failed - structured message from the agent", error: "A possum on the fridge", }; let streamMsg = mkEventStreamMsg(msgData, "unknown"); - let resultMsg = handleEventCodedError( - streamMsg as EventStreamMessageErrorCoded, - ); + let resultMsg = handleEventCodedError(streamMsg); expect(resultMsg).toBe("Unknown error: A possum on the fridge"); - streamMsg = mkEventStreamMsg(msgData, "deploymentTargetNotFound"); - resultMsg = handleEventCodedError( - streamMsg as EventStreamMessageErrorCoded, - ); - expect(resultMsg).toBe( - `Content at https://here.it.is/content/abcdefg could not be found. Please, verify the content "id" is accurate.`, - ); - - streamMsg = mkEventStreamMsg(msgData, "deploymentTargetIsForbidden"); - resultMsg = handleEventCodedError( - streamMsg as EventStreamMessageErrorCoded, - ); + streamMsg = mkEventStreamMsg(msgData, "deployFailed"); + resultMsg = handleEventCodedError(streamMsg); expect(resultMsg).toBe( - "You don't have enough permissions to deploy to https://here.it.is/content/abcdefg. Please, verify the credentials in use.", + "Deployment failed - structured message from the agent", ); }); }); diff --git a/extensions/vscode/src/eventErrors.ts b/extensions/vscode/src/eventErrors.ts index 0f68a7c40..840cf78e0 100644 --- a/extensions/vscode/src/eventErrors.ts +++ b/extensions/vscode/src/eventErrors.ts @@ -17,30 +17,23 @@ export function isCodedEventErrorMessage( type deploymentEvtErr = { dashboardUrl: string; localId: string; + message: string; error: string; }; -export const isEvtErrTargetNotFound = ( +export const isEvtErrDeploymentFailed = ( emsg: EventStreamMessageErrorCoded, ): emsg is EventStreamMessageErrorCoded => { - return emsg.errCode === "deploymentTargetNotFound"; -}; - -export const isEvtErrTargetForbidden = ( - emsg: EventStreamMessageErrorCoded, -): emsg is EventStreamMessageErrorCoded => { - return emsg.errCode === "deploymentTargetIsForbidden"; + return emsg.errCode === "deployFailed"; }; export const handleEventCodedError = ( emsg: EventStreamMessageErrorCoded, ): string => { - if (isEvtErrTargetNotFound(emsg)) { - return `Content at ${emsg.data.dashboardUrl} could not be found. Please, verify the content "id" is accurate.`; - } - if (isEvtErrTargetForbidden(emsg)) { - return `You don't have enough permissions to deploy to ${emsg.data.dashboardUrl}. Please, verify the credentials in use.`; + if (isEvtErrDeploymentFailed(emsg)) { + return emsg.data.message; } + const unknownErrMsg = emsg.data.error || emsg.data.message; return `Unknown error: ${unknownErrMsg}`; }; diff --git a/extensions/vscode/src/utils/errorTypes.ts b/extensions/vscode/src/utils/errorTypes.ts index fd813d4e7..0c35a6580 100644 --- a/extensions/vscode/src/utils/errorTypes.ts +++ b/extensions/vscode/src/utils/errorTypes.ts @@ -8,9 +8,7 @@ export type ErrorCode = | "invalidTOML" | "unknownTOMLKey" | "invalidConfigFile" - | "deploymentTargetNotFound" - | "deploymentTargetIsForbidden" - | "deploymentTargetUnreachable"; + | "deployFailed"; export type axiosErrorWithJson = AxiosError & { diff --git a/internal/clients/connect/content.go b/internal/clients/connect/content.go index 5c4f32ba7..06360988c 100644 --- a/internal/clients/connect/content.go +++ b/internal/clients/connect/content.go @@ -3,6 +3,8 @@ package connect // Copyright (C) 2023 by Posit Software, PBC. import ( + "fmt" + "github.com/posit-dev/publisher/internal/config" "github.com/posit-dev/publisher/internal/types" ) @@ -10,6 +12,7 @@ import ( type ConnectContent struct { Name types.ContentName `json:"name"` Title string `json:"title,omitempty"` + GUID string `json:"guid,omitempty"` Description string `json:"description,omitempty"` AccessType string `json:"access_type,omitempty"` ConnectionTimeout *int32 `json:"connection_timeout,omitempty"` @@ -32,6 +35,15 @@ type ConnectContent struct { DefaultImageName string `json:"default_image_name,omitempty"` DefaultREnvironmentManagement *bool `json:"default_r_environment_management,omitempty"` DefaultPyEnvironmentManagement *bool `json:"default_py_environment_management,omitempty"` + Locked bool `json:"locked,omitempty"` +} + +// Returns and error if content is locked +func (c *ConnectContent) LockedError() error { + if c.Locked { + return fmt.Errorf("content with ID %s is locked", c.GUID) + } + return nil } func copy[T any](src *T) *T { diff --git a/internal/clients/connect/mock_client.go b/internal/clients/connect/mock_client.go index 94b028666..56875bda0 100644 --- a/internal/clients/connect/mock_client.go +++ b/internal/clients/connect/mock_client.go @@ -38,6 +38,11 @@ func (m *MockClient) TestAuthentication(log logging.Logger) (*User, error) { } func (m *MockClient) ContentDetails(id types.ContentID, s *ConnectContent, log logging.Logger) error { + // Updates content as locked when needed + if id == "myLockedContentID" { + s.GUID = "myLockedContentID" + s.Locked = true + } args := m.Called(id, s, log) return args.Error(0) } diff --git a/internal/clients/http_client/http_client_test.go b/internal/clients/http_client/http_client_test.go index 244a2d812..6e698e402 100644 --- a/internal/clients/http_client/http_client_test.go +++ b/internal/clients/http_client/http_client_test.go @@ -5,6 +5,7 @@ import ( "net/http" "testing" + "github.com/posit-dev/publisher/internal/events" "github.com/posit-dev/publisher/internal/types" "github.com/posit-dev/publisher/internal/util/utiltest" "github.com/stretchr/testify/suite" @@ -22,7 +23,7 @@ func TestHttpClientSuite(t *testing.T) { func (s *HttpClientSuite) TestIsHTTPAgentErrorStatusOf() { agentErr := types.NewAgentError( - types.ErrorDeploymentTargetNotFound, + events.DeploymentFailedCode, NewHTTPError("", "", http.StatusNotFound), nil, ) diff --git a/internal/publish/publish.go b/internal/publish/publish.go index a7927f9c5..05107febe 100644 --- a/internal/publish/publish.go +++ b/internal/publish/publish.go @@ -127,7 +127,7 @@ func (p *defaultPublisher) emitErrorEvents(err error) { var data events.EventData mapstructure.Decode(publishFailureData{ - Message: agentErr.Error(), + Message: agentErr.Message, }, &data) // Record the error in the deployment record @@ -267,25 +267,46 @@ func (p *defaultPublisher) createDeploymentRecord( return p.writeDeploymentRecord() } +// Handle preflight agent error details +func (p *defaultPublisher) preflightAgentError(agenterr *types.AgentError, contentID types.ContentID) *types.AgentError { + agenterr.Code = events.DeploymentFailedCode + + if aerr, isNotFound := http_client.IsHTTPAgentErrorStatusOf(agenterr, http.StatusNotFound); isNotFound { + p.log.Debug("Content cannot be found. Deployment cannot proceed", "content_id", contentID) + aerr.Message = fmt.Sprintf("Cannot deploy content: ID %s - Content cannot be found", contentID) + } else if aerr, isForbidden := http_client.IsHTTPAgentErrorStatusOf(agenterr, http.StatusForbidden); isForbidden { + p.log.Debug("Insufficient permissions. Content deployment cannot proceed", "content_id", contentID) + aerr.Message = fmt.Sprintf("Cannot deploy content: ID %s - You may need to request collaborator permissions or verify the credentials in use", contentID) + } else { + p.log.Debug("Unknown error while deploying", "content_id", contentID) + aerr.Message = fmt.Sprintf("Cannot deploy content: ID %s - Unknown error: %s", contentID, agenterr.Error()) + } + + return agenterr +} + // Does the target exist and the user has permissions to it? func (p *defaultPublisher) targetPreflightChecks(client connect.APIClient, contentID types.ContentID) error { p.log.Debug("Running deployment preflight checks", "content_id", contentID) content := &connect.ConnectContent{} err := client.ContentDetails(contentID, content, p.log) + + // Handle possible errors from pinging to the content item + if aerr, isAgentErr := types.IsAgentError(err); isAgentErr { + return p.preflightAgentError(aerr, contentID) + } if err != nil { - if aerr, isForbidden := http_client.IsHTTPAgentErrorStatusOf(err, http.StatusForbidden); isForbidden { - p.log.Debug("Insufficient permissions. Content deployment cannot proceed", "content_id", contentID) - aerr.Code = types.ErrorDeploymentTargetIsForbidden - return aerr - } - if aerr, isNotFound := http_client.IsHTTPAgentErrorStatusOf(err, http.StatusNotFound); isNotFound { - p.log.Debug("Content cannot be found. Deployment cannot proceed", "content_id", contentID) - aerr.Code = types.ErrorDeploymentTargetNotFound - return aerr - } p.log.Debug("Deployment target cannot be reached. Halting deployment", "content_id", contentID) - return types.NewAgentError(types.ErrorDeploymentTargetUnreachable, err, nil) + return types.NewAgentError(events.DeploymentFailedCode, err, nil) } + + // Verify content is not locked + lockedErr := content.LockedError() + if lockedErr != nil { + p.log.Debug("Content is locked, cannot deploy to it", "content_id", contentID) + return types.NewAgentError(events.DeploymentFailedCode, lockedErr, nil) + } + return nil } diff --git a/internal/publish/publish_test.go b/internal/publish/publish_test.go index 964bdf707..29a73bd2c 100644 --- a/internal/publish/publish_test.go +++ b/internal/publish/publish_test.go @@ -4,6 +4,7 @@ package publish import ( "bytes" "errors" + "fmt" "log/slog" "net/http" "strings" @@ -193,37 +194,58 @@ func (s *PublishSuite) TestPublishWithClientRedeployFailCapabilities() { s.publishWithClient(target, &publishErrsMock{capErr: capErr}, capErr) } -func (s *PublishSuite) TestPublishWithClientRedeployNoPermissionsErr() { +func (s *PublishSuite) TestPublishWithClientRedeployErrors() { target := deployment.New() target.ID = "myContentID" + + // Forbidden checksErr := types.NewAgentError( - types.ErrorDeploymentTargetIsForbidden, + events.DeploymentFailedCode, http_client.NewHTTPError("", "", http.StatusForbidden), nil, ) s.publishWithClient(target, &publishErrsMock{checksErr: checksErr}, checksErr) -} + s.Equal( + checksErr.Message, + "Cannot deploy content: ID myContentID - You may need to request collaborator permissions or verify the credentials in use", + ) -func (s *PublishSuite) TestPublishWithClientRedeployContentNotFound() { - target := deployment.New() - target.ID = "myContentID" - checksErr := types.NewAgentError( - types.ErrorDeploymentTargetNotFound, + // Not Found + checksErr = types.NewAgentError( + events.DeploymentFailedCode, http_client.NewHTTPError("", "", http.StatusNotFound), nil, ) s.publishWithClient(target, &publishErrsMock{checksErr: checksErr}, checksErr) -} + s.Equal( + checksErr.Message, + "Cannot deploy content: ID myContentID - Content cannot be found", + ) -func (s *PublishSuite) TestPublishWithClientRedeployCannotReachContentTarget() { - target := deployment.New() - target.ID = "myContentID" - checksErr := types.NewAgentError( - types.ErrorDeploymentTargetUnreachable, - http_client.NewHTTPError("", "", http.StatusServiceUnavailable), + // Unknown err + checksErr = types.NewAgentError( + events.DeploymentFailedCode, + http_client.NewHTTPError("", "", http.StatusBadGateway), nil, ) s.publishWithClient(target, &publishErrsMock{checksErr: checksErr}, checksErr) + s.Equal( + checksErr.Message, + "Cannot deploy content: ID myContentID - Unknown error: unexpected response from the server (502)", + ) + + // Content is locked + target.ID = "myLockedContentID" + checksErr = types.NewAgentError( + events.DeploymentFailedCode, + fmt.Errorf("content with ID %s is locked", target.ID), + nil, + ) + s.publishWithClient(target, &publishErrsMock{}, checksErr) + s.Equal( + checksErr.Message, + "content with ID myLockedContentID is locked", + ) } func (s *PublishSuite) TestPublishWithClientRedeployFailUpdate() { @@ -278,6 +300,7 @@ func (s *PublishSuite) publishWithClient( } myContentID := types.ContentID("myContentID") + myLockedContentID := types.ContentID("myLockedContentID") myBundleID := types.BundleID("myBundleID") myTaskID := types.TaskID("myTaskID") @@ -288,6 +311,7 @@ func (s *PublishSuite) publishWithClient( client.On("TestAuthentication", mock.Anything).Return(&connect.User{}, errsMock.authErr) client.On("CheckCapabilities", mock.Anything, mock.Anything, mock.Anything).Return(errsMock.capErr) client.On("ContentDetails", myContentID, mock.Anything, mock.Anything).Return(errsMock.checksErr) + client.On("ContentDetails", myLockedContentID, mock.Anything, mock.Anything).Return(errsMock.checksErr) client.On("UpdateDeployment", myContentID, mock.Anything, mock.Anything).Return(errsMock.createErr) client.On("SetEnvVars", myContentID, mock.Anything, mock.Anything).Return(errsMock.envVarErr) client.On("UploadBundle", myContentID, mock.Anything, mock.Anything).Return(myBundleID, errsMock.uploadErr) @@ -380,13 +404,17 @@ func (s *PublishSuite) publishWithClient( record, err := deployment.FromFile(recordPath) s.NoError(err) - s.Equal(myContentID, record.ID) + if target != nil && target.ID == myLockedContentID { + s.Equal(myLockedContentID, record.ID) + } else { + s.Equal(myContentID, record.ID) + } s.Equal(project.Version, record.ClientVersion) s.NotEqual("", record.DeployedAt) s.Equal("myConfig", record.ConfigName) s.NotNil(record.Configuration) - if couldCreateDeployment { + if couldCreateDeployment && record.ID == myContentID { logs := s.logBuffer.String() s.Contains(logs, "content_id="+myContentID) s.Equal("https://connect.example.com/connect/#/apps/myContentID", record.DashboardURL) diff --git a/internal/types/error.go b/internal/types/error.go index 8213023f2..9d6e52570 100644 --- a/internal/types/error.go +++ b/internal/types/error.go @@ -16,9 +16,6 @@ const ( ErrorUnknownTOMLKey ErrorCode = "unknownTOMLKey" ErrorInvalidConfigFiles ErrorCode = "invalidConfigFiles" ErrorCredentialServiceUnavailable ErrorCode = "credentialsServiceUnavailable" - ErrorDeploymentTargetNotFound ErrorCode = "deploymentTargetNotFound" - ErrorDeploymentTargetIsForbidden ErrorCode = "deploymentTargetIsForbidden" - ErrorDeploymentTargetUnreachable ErrorCode = "deploymentTargetUnreachable" ErrorUnknown ErrorCode = "unknown" ) @@ -102,3 +99,13 @@ func OperationError(op Operation, err error) EventableError { e.SetOperation(op) return e } + +// Evaluate if a given error is an AgentError +// returning the error as AgentError type when it is +// and a bool flag of the comparison result. +func IsAgentError(err error) (*AgentError, bool) { + if aerr, ok := err.(*AgentError); ok { + return aerr, ok + } + return nil, false +} diff --git a/internal/types/error_test.go b/internal/types/error_test.go index 56c5ac0b5..0b4de2011 100644 --- a/internal/types/error_test.go +++ b/internal/types/error_test.go @@ -67,3 +67,20 @@ func (s *ErrorSuite) TestNewAgentError() { Data: make(ErrorData), }) } + +func (s *ErrorSuite) TestIsAgentError() { + originalError := errors.New("shattered glass!") + aerr, isIt := IsAgentError(originalError) + s.Equal(isIt, false) + s.Nil(aerr) + + aTrueAgentError := NewAgentError(ErrorInvalidTOML, originalError, nil) + aerr, isIt = IsAgentError(aTrueAgentError) + s.Equal(isIt, true) + s.Equal(aerr, &AgentError{ + Message: "shattered glass!", + Code: ErrorInvalidTOML, + Err: originalError, + Data: make(ErrorData), + }) +}