Skip to content

Commit

Permalink
AgentError messages punctuation helper
Browse files Browse the repository at this point in the history
  • Loading branch information
marcosnav committed Oct 11, 2024
1 parent c67ffdb commit a7429cb
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 11 deletions.
5 changes: 2 additions & 3 deletions internal/clients/connect/capabilities.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
Expand All @@ -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
Expand Down
4 changes: 2 additions & 2 deletions internal/clients/connect/client_connect.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
Expand Down
2 changes: 1 addition & 1 deletion internal/clients/connect/client_connect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion internal/services/api/get_deployment_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion internal/services/api/post_test_credentials_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
18 changes: 17 additions & 1 deletion internal/types/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ package types
// Copyright (C) 2023 by Posit Software, PBC.

import (
"slices"
"strings"

"github.com/mitchellh/mapstructure"
)

Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down
46 changes: 44 additions & 2 deletions internal/types/error_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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),
})
}

0 comments on commit a7429cb

Please sign in to comment.