diff --git a/internal/clients/connect/capabilities.go b/internal/clients/connect/capabilities.go index edaa171a0..6384640ef 100644 --- a/internal/clients/connect/capabilities.go +++ b/internal/clients/connect/capabilities.go @@ -25,7 +25,7 @@ type allSettings struct { quarto server_settings.QuartoInfo } -const requirementsFileMissing = `Missing dependency file %s. This file must be included in the deployment.` +const requirementsFileMissing = `missing dependency file %s. This file must be included in the deployment` type requirementsErrDetails struct { RequirementsFile string `json:"requirements_file"` @@ -51,9 +51,8 @@ func checkRequirementsFile(base util.AbsolutePath, cfg *config.Config) error { } if !exists || !requirementsIsIncluded { - missingErr := fmt.Errorf("missing dependency file %s", cfg.Python.PackageFile) + missingErr := fmt.Errorf(requirementsFileMissing, cfg.Python.PackageFile) aerr := types.NewAgentError(types.ErrorRequirementsFileReading, missingErr, requirementsErrDetails{RequirementsFile: packageFile.String()}) - aerr.Message = fmt.Sprintf(requirementsFileMissing, cfg.Python.PackageFile) return aerr } return nil diff --git a/internal/clients/connect/client_connect.go b/internal/clients/connect/client_connect.go index db079bba1..1bfe629ab 100644 --- a/internal/clients/connect/client_connect.go +++ b/internal/clients/connect/client_connect.go @@ -492,9 +492,9 @@ func (c *ConnectClient) preflightAgentError(agenterr *types.AgentError, contentI agenterr.Code = events.DeploymentFailedCode if _, isNotFound := http_client.IsHTTPAgentErrorStatusOf(agenterr, http.StatusNotFound); isNotFound { - agenterr.Message = fmt.Sprintf("Cannot deploy content: ID %s - Content cannot be found", contentID) + agenterr.Message = fmt.Sprintf("Cannot deploy content: ID %s - Content cannot be found.", contentID) } else if _, isForbidden := http_client.IsHTTPAgentErrorStatusOf(agenterr, http.StatusForbidden); isForbidden { - agenterr.Message = fmt.Sprintf("Cannot deploy content: ID %s - You may need to request collaborator permissions or verify the credentials in use", contentID) + agenterr.Message = fmt.Sprintf("Cannot deploy content: ID %s - You may need to request collaborator permissions or verify the credentials in use.", contentID) } else { agenterr.Message = fmt.Sprintf("Cannot deploy content: ID %s - Unknown error: %s", contentID, agenterr.Error()) } diff --git a/internal/clients/connect/client_connect_test.go b/internal/clients/connect/client_connect_test.go index 4cf743c93..1fe13e4aa 100644 --- a/internal/clients/connect/client_connect_test.go +++ b/internal/clients/connect/client_connect_test.go @@ -668,7 +668,7 @@ func (s *ConnectClientSuite) TestValidateDeploymentTargetNotFoundFailure() { } expectedErr := types.NewAgentError( events.DeploymentFailedCode, - errors.New("Cannot deploy content: ID e8922765-4880-43cd-abc0-d59fe59b8b4b - Content cannot be found"), + errors.New("cannot deploy content: ID e8922765-4880-43cd-abc0-d59fe59b8b4b - Content cannot be found"), nil) err := client.ValidateDeploymentTarget("e8922765-4880-43cd-abc0-d59fe59b8b4b", s.cfg, lgr) aerr, ok := types.IsAgentError(err) diff --git a/internal/services/api/get_deployment_test.go b/internal/services/api/get_deployment_test.go index 22056a27f..de7ccdb1e 100644 --- a/internal/services/api/get_deployment_test.go +++ b/internal/services/api/get_deployment_test.go @@ -138,7 +138,7 @@ func (s *GetDeploymentSuite) TestGetPreDeployment() { dec.DisallowUnknownFields() s.NoError(dec.Decode(&res)) s.NotNil(res.Error) - s.Equal("test error", res.Error.Message) + s.Equal("Test error.", res.Error.Message) s.Equal(deploymentStateNew, res.State) s.Equal("myTargetName", res.Name) s.Equal(".", res.ProjectDir) diff --git a/internal/services/api/post_test_credentials_test.go b/internal/services/api/post_test_credentials_test.go index 6eee08636..72a6d254c 100644 --- a/internal/services/api/post_test_credentials_test.go +++ b/internal/services/api/post_test_credentials_test.go @@ -124,5 +124,5 @@ func (s *PostTestCredentialsHandlerSuite) TestPostTestCredentialsHandlerFuncBadA s.NoError(err) s.Nil(response.User) s.NotNil(response.Error) - s.Equal("test error from TestAuthentication", response.Error.Message) + s.Equal("Test error from TestAuthentication.", response.Error.Message) } diff --git a/internal/types/error.go b/internal/types/error.go index 4ddd13db5..c8eaa4ff9 100644 --- a/internal/types/error.go +++ b/internal/types/error.go @@ -3,6 +3,9 @@ package types // Copyright (C) 2023 by Posit Software, PBC. import ( + "slices" + "strings" + "github.com/mitchellh/mapstructure" ) @@ -39,6 +42,19 @@ type AgentError struct { Data ErrorData `json:"data" toml:"data,omitempty"` } +// Normalize punctuation on messages derived from errors +func normalizeAgentErrorMsg(errMsg string) string { + spChars := []string{"?", "!", ")", "."} + msg := strings.ToUpper(errMsg[:1]) + errMsg[1:] + + msgLastChar := msg[len(msg)-1:] + if slices.Contains(spChars, msgLastChar) { + return msg + } + + return msg + "." +} + func AsAgentError(e error) *AgentError { if e == nil { return nil @@ -55,7 +71,7 @@ func NewAgentError(code ErrorCode, err error, details any) *AgentError { msg := "" if err != nil { - msg = err.Error() + msg = normalizeAgentErrorMsg(err.Error()) } if details != nil { detailMap := make(ErrorData) diff --git a/internal/types/error_test.go b/internal/types/error_test.go index 0b4de2011..9dbf55f3f 100644 --- a/internal/types/error_test.go +++ b/internal/types/error_test.go @@ -61,7 +61,7 @@ func (s *ErrorSuite) TestNewAgentError() { originalError := errors.New("shattered glass!") aerr := NewAgentError(ErrorInvalidTOML, originalError, nil) s.Equal(aerr, &AgentError{ - Message: "shattered glass!", + Message: "Shattered glass!", Code: ErrorInvalidTOML, Err: originalError, Data: make(ErrorData), @@ -78,9 +78,51 @@ func (s *ErrorSuite) TestIsAgentError() { aerr, isIt = IsAgentError(aTrueAgentError) s.Equal(isIt, true) s.Equal(aerr, &AgentError{ - Message: "shattered glass!", + Message: "Shattered glass!", Code: ErrorInvalidTOML, Err: originalError, Data: make(ErrorData), }) } + +func (s *ErrorSuite) TestNewAgentError_MessagePunctuation() { + // Sentence case and period ending + originalError := errors.New("oh sorry, my mistake") + aerr := NewAgentError(ErrorResourceNotFound, originalError, nil) + s.Equal(aerr, &AgentError{ + Message: "Oh sorry, my mistake.", + Code: ErrorResourceNotFound, + Err: originalError, + Data: make(ErrorData), + }) + + // No need for period ending when shouting "!" + originalError = errors.New("i can't believe is October already!") + aerr = NewAgentError(ErrorResourceNotFound, originalError, nil) + s.Equal(aerr, &AgentError{ + Message: "I can't believe is October already!", + Code: ErrorResourceNotFound, + Err: originalError, + Data: make(ErrorData), + }) + + // No need for period ending when asking "?" + originalError = errors.New("can you believe 2024 is almos over?") + aerr = NewAgentError(ErrorResourceNotFound, originalError, nil) + s.Equal(aerr, &AgentError{ + Message: "Can you believe 2024 is almos over?", + Code: ErrorResourceNotFound, + Err: originalError, + Data: make(ErrorData), + }) + + // No need for period ending when ending parentheses ")" + originalError = errors.New("wrong credentials (is this one of those bad typing days?)") + aerr = NewAgentError(ErrorResourceNotFound, originalError, nil) + s.Equal(aerr, &AgentError{ + Message: "Wrong credentials (is this one of those bad typing days?)", + Code: ErrorResourceNotFound, + Err: originalError, + Data: make(ErrorData), + }) +}