Skip to content

Commit

Permalink
Rewrite places we use non-constant format strings
Browse files Browse the repository at this point in the history
  • Loading branch information
rowanseymour committed Feb 27, 2025
1 parent 630e8fc commit 32be343
Show file tree
Hide file tree
Showing 28 changed files with 71 additions and 74 deletions.
2 changes: 1 addition & 1 deletion cmd/flowrunner/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func TestPrintEvent(t *testing.T) {
{events.NewDialWait(urns.URN(`tel:+1234567890`), 20, 120, expiresOn), `⏳ waiting for dial (type /dial <answered|no_answer|busy|failed>)...`},
{events.NewEmailSent([]string{"code@example.com"}, "Hi", "What up?"), `✉️ email sent with subject 'Hi'`},
{events.NewEnvironmentRefreshed(session.Environment()), `⚙️ environment refreshed on resume`},
{events.NewErrorf("this didn't work"), `⚠️ this didn't work`},
{events.NewError("this didn't work"), `⚠️ this didn't work`},
{events.NewFailure(errors.New("this really didn't work")), `🛑 this really didn't work`},
{events.NewFlowEntered(flow.Reference(false), "", false), `↪️ entered flow 'Registration'`},
{events.NewInputLabelsAdded("2a786bbc-2314-4d57-a0c9-b66e1642e5e2", []*flows.Label{sa.Labels().FindByName("Spam")}), `🏷️ labeled with 'Spam'`},
Expand Down
5 changes: 2 additions & 3 deletions contactql/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package contactql

import (
"errors"
"fmt"
)

// error codes with values included in extra
Expand Down Expand Up @@ -32,8 +31,8 @@ type QueryError struct {
}

// NewQueryError creates a new query error
func NewQueryError(code, err string, args ...any) *QueryError {
return &QueryError{code: code, msg: fmt.Sprintf(err, args...)}
func NewQueryError(code, msg string) *QueryError {
return &QueryError{code: code, msg: msg}
}

func (e *QueryError) withExtra(k string, v any) *QueryError {
Expand Down
22 changes: 11 additions & 11 deletions contactql/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,18 +143,18 @@ func (c *Condition) validate(env envs.Environment, resolver Resolver) error {

valueType := c.resolveValueType(resolver)
if valueType == "" {
return NewQueryError(ErrUnknownProperty, "can't resolve '%s' to attribute, scheme or field", c.propKey).withExtra("property", c.propKey)
return NewQueryError(ErrUnknownProperty, fmt.Sprintf("can't resolve '%s' to attribute, scheme or field", c.propKey)).withExtra("property", c.propKey)
}

switch c.operator {
case OpContains:
if c.propKey == AttributeName {
if len(tokenizeNameValue(c.value)) == 0 {
return NewQueryError(ErrInvalidPartialName, "contains operator on name requires token of minimum length %d", minNameTokenContainsLength).withExtra("min_token_length", strconv.Itoa(minNameTokenContainsLength))
return NewQueryError(ErrInvalidPartialName, fmt.Sprintf("contains operator on name requires token of minimum length %d", minNameTokenContainsLength)).withExtra("min_token_length", strconv.Itoa(minNameTokenContainsLength))
}
} else if c.propKey == AttributeURN || c.propType == PropertyTypeURN {
if len(c.value) < minURNContainsLength {
return NewQueryError(ErrInvalidPartialURN, "contains operator on URN requires value of minimum length %d", minURNContainsLength).withExtra("min_value_length", strconv.Itoa(minURNContainsLength))
return NewQueryError(ErrInvalidPartialURN, fmt.Sprintf("contains operator on URN requires value of minimum length %d", minURNContainsLength)).withExtra("min_value_length", strconv.Itoa(minURNContainsLength))
}
} else {
// ~ can only be used with the name/urn attributes or actual URNs
Expand All @@ -163,27 +163,27 @@ func (c *Condition) validate(env envs.Environment, resolver Resolver) error {

case OpGreaterThan, OpGreaterThanOrEqual, OpLessThan, OpLessThanOrEqual:
if valueType != assets.FieldTypeNumber && valueType != assets.FieldTypeDatetime {
return NewQueryError(ErrUnsupportedComparison, "comparisons with %s can only be used with date and number fields", c.operator).withExtra("property", c.propKey).withExtra("operator", string(c.operator))
return NewQueryError(ErrUnsupportedComparison, fmt.Sprintf("comparisons with %s can only be used with date and number fields", c.operator)).withExtra("property", c.propKey).withExtra("operator", string(c.operator))
}
}

// if existence check, disallow certain attributes
if (c.operator == OpEqual || c.operator == OpNotEqual) && c.value == "" {
switch c.propKey {
case AttributeUUID, AttributeID, AttributeStatus, AttributeCreatedOn, AttributeTickets:
return NewQueryError(ErrUnsupportedSetCheck, "can't check whether '%s' is set or not set", c.propKey).withExtra("property", c.propKey).withExtra("operator", string(c.operator))
return NewQueryError(ErrUnsupportedSetCheck, fmt.Sprintf("can't check whether '%s' is set or not set", c.propKey)).withExtra("property", c.propKey).withExtra("operator", string(c.operator))
}
} else {
// check values are valid for the value type
if valueType == assets.FieldTypeNumber {
_, err := c.ValueAsNumber()
if err != nil {
return NewQueryError(ErrInvalidNumber, "can't convert '%s' to a number", c.value).withExtra("value", c.value)
return NewQueryError(ErrInvalidNumber, fmt.Sprintf("can't convert '%s' to a number", c.value)).withExtra("value", c.value)
}
} else if valueType == assets.FieldTypeDatetime {
_, err := c.ValueAsDate(env)
if err != nil {
return NewQueryError(ErrInvalidDate, "can't convert '%s' to a date", c.value).withExtra("value", c.value)
return NewQueryError(ErrInvalidDate, fmt.Sprintf("can't convert '%s' to a date", c.value)).withExtra("value", c.value)
}
}

Expand All @@ -192,23 +192,23 @@ func (c *Condition) validate(env envs.Environment, resolver Resolver) error {
if c.propKey == AttributeGroup && resolver != nil {
group := c.ValueAsGroup(resolver)
if group == nil {
return NewQueryError(ErrInvalidGroup, "'%s' is not a valid group name", c.value).withExtra("value", c.value)
return NewQueryError(ErrInvalidGroup, fmt.Sprintf("'%s' is not a valid group name", c.value)).withExtra("value", c.value)
}
} else if (c.propKey == AttributeFlow || c.propKey == AttributeHistory) && resolver != nil {
flow := c.ValueAsFlow(resolver)
if flow == nil {
return NewQueryError(ErrInvalidFlow, "'%s' is not a valid flow name", c.value).withExtra("value", c.value)
return NewQueryError(ErrInvalidFlow, fmt.Sprintf("'%s' is not a valid flow name", c.value)).withExtra("value", c.value)
}
} else if c.propKey == AttributeStatus {
val := strings.ToLower(c.value)
if val != "active" && val != "blocked" && val != "stopped" && val != "archived" {
return NewQueryError(ErrInvalidStatus, "'%s' is not a valid contact status", c.value).withExtra("value", c.value)
return NewQueryError(ErrInvalidStatus, fmt.Sprintf("'%s' is not a valid contact status", c.value)).withExtra("value", c.value)
}
} else if c.propKey == AttributeLanguage {
if c.value != "" {
_, err := i18n.ParseLanguage(c.value)
if err != nil {
return NewQueryError(ErrInvalidLanguage, "'%s' is not a valid language code", c.value).withExtra("value", c.value)
return NewQueryError(ErrInvalidLanguage, fmt.Sprintf("'%s' is not a valid language code", c.value)).withExtra("value", c.value)
}
}
}
Expand Down
3 changes: 2 additions & 1 deletion contactql/visitor.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package contactql

import (
"fmt"
"regexp"
"strconv"
"strings"
Expand Down Expand Up @@ -139,7 +140,7 @@ func (v *visitor) VisitCondition(ctx *gen.ConditionContext) any {
propType = PropertyTypeURN
propKey = parts[1]
} else {
v.addError(NewQueryError(ErrUnknownPropertyType, "unknown property type '%s'", parts[0]).withExtra("type", parts[0]))
v.addError(NewQueryError(ErrUnknownPropertyType, fmt.Sprintf("unknown property type '%s'", parts[0])).withExtra("type", parts[0]))
}
} else {
propKey = propText
Expand Down
2 changes: 1 addition & 1 deletion flows/actions/add_contact_urn.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (a *AddContactURNAction) Execute(run flows.Run, step flows.Step, logModifie
evaluatedPath, _ := run.EvaluateTemplate(a.Path, logEvent)
evaluatedPath = strings.TrimSpace(evaluatedPath)
if evaluatedPath == "" {
logEvent(events.NewErrorf("can't add URN with empty path"))
logEvent(events.NewError("can't add URN with empty path"))
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion flows/actions/add_input_labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (a *AddInputLabelsAction) Execute(run flows.Run, step flows.Step, logModifi
// log error if we don't have any input that could be labeled
input := run.Session().Input()
if input == nil {
logEvent(events.NewErrorf("no input to add labels to"))
logEvent(events.NewError("no input to add labels to"))
return nil
}

Expand Down
14 changes: 7 additions & 7 deletions flows/actions/base.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,11 +87,11 @@ func (a *baseAction) evaluateMessage(run flows.Run, languages []i18n.Language, a
evaluatedAttachment, _ := run.EvaluateTemplate(a, logEvent)
evaluatedAttachment = strings.TrimSpace(evaluatedAttachment)
if !utils.IsValidAttachment(evaluatedAttachment) {
logEvent(events.NewErrorf("attachment evaluated to invalid value, skipping"))
logEvent(events.NewError("attachment evaluated to invalid value, skipping"))
continue
}
if len(evaluatedAttachment) > flows.MaxAttachmentLength {
logEvent(events.NewErrorf("evaluated attachment is longer than %d limit, skipping", flows.MaxAttachmentLength))
logEvent(events.NewError(fmt.Sprintf("evaluated attachment is longer than %d limit, skipping", flows.MaxAttachmentLength)))
continue
}
evaluatedAttachments = append(evaluatedAttachments, utils.Attachment(evaluatedAttachment))
Expand All @@ -103,7 +103,7 @@ func (a *baseAction) evaluateMessage(run flows.Run, languages []i18n.Language, a
for _, qr := range translatedQuickReplies {
evaluatedQuickReply, _ := run.EvaluateTemplate(qr, logEvent)
if evaluatedQuickReply == "" {
logEvent(events.NewErrorf("quick reply evaluated to empty string, skipping"))
logEvent(events.NewError("quick reply evaluated to empty string, skipping"))
continue
}
evaluatedQuickReplies = append(evaluatedQuickReplies, flows.QuickReply{Text: stringsx.TruncateEllipsis(evaluatedQuickReply, flows.MaxQuickReplyLength)})
Expand Down Expand Up @@ -248,7 +248,7 @@ func (a *otherContactsAction) resolveRecipients(run flows.Run, logEvent flows.Ev
urn, _ := urns.New(urns.Phone, parsedTel)
urnList = append(urnList, urn)
} else {
logEvent(events.NewErrorf("'%s' couldn't be resolved to a contact, group or URN", evaluatedLegacyVar))
logEvent(events.NewError(fmt.Sprintf("'%s' couldn't be resolved to a contact, group or URN", evaluatedLegacyVar)))
}
}
}
Expand Down Expand Up @@ -284,7 +284,7 @@ func resolveGroups(run flows.Run, references []*assets.GroupReference, logEvent
// look up the set of all groups to see if such a group exists
group = groupAssets.FindByName(evaluatedName)
if group == nil {
logEvent(events.NewErrorf("no such group with name '%s'", evaluatedName))
logEvent(events.NewError(fmt.Sprintf("no such group with name '%s'", evaluatedName)))
}
}
} else {
Expand Down Expand Up @@ -318,7 +318,7 @@ func resolveLabels(run flows.Run, references []*assets.LabelReference, logEvent
// look up the set of all labels to see if such a label exists
label = labelAssets.FindByName(evaluatedName)
if label == nil {
logEvent(events.NewErrorf("no such label with name '%s'", evaluatedName))
logEvent(events.NewError(fmt.Sprintf("no such label with name '%s'", evaluatedName)))
}
}
} else {
Expand Down Expand Up @@ -349,7 +349,7 @@ func resolveUser(run flows.Run, ref *assets.UserReference, logEvent flows.EventC
// look up to see if such a user exists
user = userAssets.Get(evaluatedEmail)
if user == nil {
logEvent(events.NewErrorf("no such user with email '%s'", evaluatedEmail))
logEvent(events.NewError(fmt.Sprintf("no such user with email '%s'", evaluatedEmail)))
}
}
} else {
Expand Down
6 changes: 3 additions & 3 deletions flows/actions/call_classifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (a *CallClassifierAction) Execute(run flows.Run, step flows.Step, logModifi

func (a *CallClassifierAction) classify(run flows.Run, input string, classifier *flows.Classifier, logEvent flows.EventCallback) (*flows.Classification, bool) {
if input == "" {
logEvent(events.NewErrorf("can't classify empty input, skipping classification"))
logEvent(events.NewError("can't classify empty input, skipping classification"))
return nil, true
}
if classifier == nil {
Expand All @@ -83,7 +83,7 @@ func (a *CallClassifierAction) classify(run flows.Run, input string, classifier

svc, err := run.Session().Engine().Services().Classification(classifier)
if err != nil {
logEvent(events.NewError(err))
logEvent(events.NewError(err.Error()))

Check warning on line 86 in flows/actions/call_classifier.go

View check run for this annotation

Codecov / codecov/patch

flows/actions/call_classifier.go#L86

Added line #L86 was not covered by tests
return nil, false
}

Expand All @@ -96,7 +96,7 @@ func (a *CallClassifierAction) classify(run flows.Run, input string, classifier
}

if err != nil {
logEvent(events.NewError(err))
logEvent(events.NewError(err.Error()))
return nil, false
}

Expand Down
6 changes: 3 additions & 3 deletions flows/actions/call_resthook.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,22 +104,22 @@ func (a *CallResthookAction) Execute(run flows.Run, step flows.Step, logModifier
for _, url := range resthook.Subscribers() {
req, err := http.NewRequest("POST", url, strings.NewReader(payload))
if err != nil {
logEvent(events.NewError(err))
logEvent(events.NewError(err.Error()))

Check warning on line 107 in flows/actions/call_resthook.go

View check run for this annotation

Codecov / codecov/patch

flows/actions/call_resthook.go#L107

Added line #L107 was not covered by tests
return nil
}

req.Header.Add("Content-Type", "application/json")

svc, err := run.Session().Engine().Services().Webhook(run.Session().Assets())
if err != nil {
logEvent(events.NewError(err))
logEvent(events.NewError(err.Error()))

Check warning on line 115 in flows/actions/call_resthook.go

View check run for this annotation

Codecov / codecov/patch

flows/actions/call_resthook.go#L115

Added line #L115 was not covered by tests
return nil
}

call, err := svc.Call(req)

if err != nil {
logEvent(events.NewError(err))
logEvent(events.NewError(err.Error()))

Check warning on line 122 in flows/actions/call_resthook.go

View check run for this annotation

Codecov / codecov/patch

flows/actions/call_resthook.go#L122

Added line #L122 was not covered by tests
}
if call != nil {
calls = append(calls, call)
Expand Down
8 changes: 4 additions & 4 deletions flows/actions/call_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,11 @@ func (a *CallWebhookAction) Execute(run flows.Run, step flows.Step, logModifier
url = strings.TrimSpace(url)

if url == "" {
logEvent(events.NewErrorf("webhook URL evaluated to empty string"))
logEvent(events.NewError("webhook URL evaluated to empty string"))
return nil
}
if !isValidURL(url) {
logEvent(events.NewErrorf("webhook URL evaluated to an invalid URL: '%s'", url))
logEvent(events.NewError(fmt.Sprintf("webhook URL evaluated to an invalid URL: '%s'", url)))
return nil
}

Expand Down Expand Up @@ -125,14 +125,14 @@ func (a *CallWebhookAction) call(run flows.Run, step flows.Step, url, method, bo

svc, err := run.Session().Engine().Services().Webhook(run.Session().Assets())
if err != nil {
logEvent(events.NewError(err))
logEvent(events.NewError(err.Error()))

Check warning on line 128 in flows/actions/call_webhook.go

View check run for this annotation

Codecov / codecov/patch

flows/actions/call_webhook.go#L128

Added line #L128 was not covered by tests
return nil
}

call, err := svc.Call(req)

if err != nil {
logEvent(events.NewError(err))
logEvent(events.NewError(err.Error()))
}
if call != nil {
run.SetWebhook(call)
Expand Down
2 changes: 1 addition & 1 deletion flows/actions/open_ticket.go
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ func (a *OpenTicketAction) Execute(run flows.Run, step flows.Step, logModifier f

func (a *OpenTicketAction) open(run flows.Run, topic *flows.Topic, assignee *flows.User, note string, logModifier flows.ModifierCallback, logEvent flows.EventCallback) *flows.Ticket {
if run.Session().BatchStart() {
logEvent(events.NewErrorf("can't open tickets during batch starts"))
logEvent(events.NewError("can't open tickets during batch starts"))
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion flows/actions/play_audio.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func (a *PlayAudioAction) Execute(run flows.Run, step flows.Step, logModifier fl

evaluatedAudioURL = strings.TrimSpace(evaluatedAudioURL)
if evaluatedAudioURL == "" {
logEvent(events.NewErrorf("audio URL evaluated to empty, skipping"))
logEvent(events.NewError("audio URL evaluated to empty, skipping"))
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion flows/actions/say_msg.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ func (a *SayMsgAction) Execute(run flows.Run, step flows.Step, logModifier flows

// if we have neither an audio URL or backdown text, skip
if evaluatedText == "" && localizedAudioURL == "" {
logEvent(events.NewErrorf("need either audio URL or backdown text, skipping"))
logEvent(events.NewError("need either audio URL or backdown text, skipping"))
return nil
}

Expand Down
2 changes: 1 addition & 1 deletion flows/actions/send_broadcast.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func (a *SendBroadcastAction) Execute(run flows.Run, step flows.Step, logModifie

// footgun prevention
if run.Session().BatchStart() && (len(groupRefs) > 0 || contactQuery != "") {
logEvent(events.NewErrorf("can't send broadcasts to groups during batch starts"))
logEvent(events.NewError("can't send broadcasts to groups during batch starts"))
return nil
}

Expand Down
10 changes: 5 additions & 5 deletions flows/actions/send_email.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,14 @@ func (a *SendEmailAction) Execute(run flows.Run, step flows.Step, logModifier fl
evaluatedSubject = strings.TrimSpace(evaluatedSubject)

if evaluatedSubject == "" {
logEvent(events.NewErrorf("email subject evaluated to empty string, skipping"))
logEvent(events.NewError("email subject evaluated to empty string, skipping"))
return nil
}

localizedBody, _ := run.GetText(uuids.UUID(a.UUID()), "body", a.Body)
evaluatedBody, _ := run.EvaluateTemplate(localizedBody, logEvent)
if evaluatedBody == "" {
logEvent(events.NewErrorf("email body evaluated to empty string, skipping"))
logEvent(events.NewError("email body evaluated to empty string, skipping"))
return nil
}

Expand All @@ -76,7 +76,7 @@ func (a *SendEmailAction) Execute(run flows.Run, step flows.Step, logModifier fl
for _, address := range a.Addresses {
evaluatedAddress, _ := run.EvaluateTemplate(address, logEvent)
if evaluatedAddress == "" {
logEvent(events.NewErrorf("email address evaluated to empty string, skipping"))
logEvent(events.NewError("email address evaluated to empty string, skipping"))
continue
}

Expand All @@ -93,13 +93,13 @@ func (a *SendEmailAction) Execute(run flows.Run, step flows.Step, logModifier fl

svc, err := run.Session().Engine().Services().Email(run.Session().Assets())
if err != nil {
logEvent(events.NewError(err))
logEvent(events.NewError(err.Error()))

Check warning on line 96 in flows/actions/send_email.go

View check run for this annotation

Codecov / codecov/patch

flows/actions/send_email.go#L96

Added line #L96 was not covered by tests
return nil
}

err = svc.Send(evaluatedAddresses, evaluatedSubject, evaluatedBody)
if err != nil {
logEvent(events.NewError(fmt.Errorf("unable to send email: %w", err)))
logEvent(events.NewError(fmt.Sprintf("unable to send email: %s", err.Error())))
} else {
logEvent(events.NewEmailSent(evaluatedAddresses, evaluatedSubject, evaluatedBody))
}
Expand Down
2 changes: 1 addition & 1 deletion flows/actions/set_contact_language.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func (a *SetContactLanguageAction) Execute(run flows.Run, step flows.Step, logMo
if language != "" {
lang, err = i18n.ParseLanguage(language)
if err != nil {
logEvent(events.NewError(err))
logEvent(events.NewError(err.Error()))
return nil
}
}
Expand Down
3 changes: 2 additions & 1 deletion flows/actions/set_contact_timezone.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package actions

import (
"fmt"
"strings"
"time"

Expand Down Expand Up @@ -57,7 +58,7 @@ func (a *SetContactTimezoneAction) Execute(run flows.Run, step flows.Step, logMo
if timezone != "" {
tz, err = time.LoadLocation(timezone)
if err != nil {
logEvent(events.NewErrorf("unrecognized timezone: '%s'", timezone))
logEvent(events.NewError(fmt.Sprintf("unrecognized timezone: '%s'", timezone)))
return nil
}
}
Expand Down
Loading

0 comments on commit 32be343

Please sign in to comment.