From 717dafc6732933990097015119e9d7caa5a8ca34 Mon Sep 17 00:00:00 2001 From: Stephan D Date: Wed, 19 Nov 2025 13:54:25 +0100 Subject: [PATCH] better message formatting --- api/notification/config.yml | 2 +- .../mail/internal/builder/builder.go | 2 +- .../notificationimp/mail/internal/mailimp.go | 2 +- .../server/notificationimp/mail/mail.go | 6 +- .../server/notificationimp/notification.go | 4 +- .../server/notificationimp/telegram/client.go | 108 +++++++++++++++--- api/pkg/auth/factory.go | 2 +- api/pkg/auth/internal/casbin/action.go | 2 +- api/pkg/auth/internal/casbin/config/config.go | 2 +- api/pkg/auth/internal/casbin/role.go | 2 +- api/pkg/auth/internal/native/role.go | 2 +- api/pkg/db/connection.go | 4 +- api/pkg/db/factory.go | 2 +- .../internal/mongo/organizationdb/create.go | 2 +- .../db/internal/mongo/refreshtokensdb/crud.go | 2 +- .../db/internal/mongo/repositoryimp/index.go | 2 +- .../mongo/repositoryimp/repository.go | 4 +- .../db/internal/mongo/tseriesimp/tseries.go | 2 +- api/pkg/merrors/errors.go | 29 ++++- api/pkg/merrors/errors_test.go | 47 ++++++++ .../messaging/internal/inprocess/broker.go | 2 +- api/pkg/messaging/internal/natsb/broker.go | 4 +- .../notifications/site/notification.go | 2 +- api/pkg/model/demorequest.go | 12 +- api/pkg/mutil/reorder/reorder.go | 4 +- api/pkg/server/grpcapp/app.go | 6 +- 26 files changed, 202 insertions(+), 56 deletions(-) create mode 100644 api/pkg/merrors/errors_test.go diff --git a/api/notification/config.yml b/api/notification/config.yml index 7e7c8d7..deae90f 100755 --- a/api/notification/config.yml +++ b/api/notification/config.yml @@ -61,7 +61,7 @@ api: thread_id_env: TELEGRAM_THREAD_ID api_url: "https://api.telegram.org" timeout_seconds: 10 - parse_mode: "" + parse_mode: markdown localizer: path: "./i18n" diff --git a/api/notification/internal/server/notificationimp/mail/internal/builder/builder.go b/api/notification/internal/server/notificationimp/mail/internal/builder/builder.go index a729130..b486c22 100644 --- a/api/notification/internal/server/notificationimp/mail/internal/builder/builder.go +++ b/api/notification/internal/server/notificationimp/mail/internal/builder/builder.go @@ -42,7 +42,7 @@ func (mb *MessageBuilderImp) AddData(key, value string) mmail.MailBuilder { func (mb *MessageBuilderImp) Build() (mmail.Message, error) { if len(mb.message.recipients) == 0 { - return nil, merrors.InvalidArgument("Recipient not set") + return nil, merrors.InvalidArgument("Recipient not set", "recipients") } return mb.message, nil } diff --git a/api/notification/internal/server/notificationimp/mail/internal/mailimp.go b/api/notification/internal/server/notificationimp/mail/internal/mailimp.go index 87f2d32..65a6df4 100755 --- a/api/notification/internal/server/notificationimp/mail/internal/mailimp.go +++ b/api/notification/internal/server/notificationimp/mail/internal/mailimp.go @@ -69,7 +69,7 @@ func (c *Client) Send(r mmail.MailBuilder) error { c.logger.Warn("Malformed messge", zap.String("template_id", m.TemplateID()), zap.String("locale", m.Locale()), zap.Strings("recipients", m.Recipients()), zap.Int("body_size", len(body))) - return merrors.InvalidArgument("malformed message") + return merrors.InvalidArgument("malformed message", "message.body", "message.recipients") } subj, err := mailkey.Subject(c.l, m.Parameters(), m.TemplateID(), m.Locale()) if err != nil { diff --git a/api/notification/internal/server/notificationimp/mail/mail.go b/api/notification/internal/server/notificationimp/mail/mail.go index 6cacb60..12f616d 100644 --- a/api/notification/internal/server/notificationimp/mail/mail.go +++ b/api/notification/internal/server/notificationimp/mail/mail.go @@ -1,6 +1,7 @@ package mail import ( + "github.com/mitchellh/mapstructure" "github.com/tech/sendico/notification/interface/api/localizer" notification "github.com/tech/sendico/notification/interface/services/notification/config" mi "github.com/tech/sendico/notification/internal/server/notificationimp/mail/internal" @@ -9,7 +10,6 @@ import ( "github.com/tech/sendico/pkg/merrors" "github.com/tech/sendico/pkg/messaging" "github.com/tech/sendico/pkg/mlogger" - "github.com/mitchellh/mapstructure" "go.uber.org/zap" ) @@ -22,7 +22,7 @@ type Config = notification.Config func createMailClient(logger mlogger.Logger, producer messaging.Producer, l localizer.Localizer, dp domainprovider.DomainProvider, config *Config) (Client, error) { if len(config.Driver) == 0 { - return nil, merrors.InvalidArgument("Mail driver name must be provided") + return nil, merrors.InvalidArgument("Mail driver name must be provided", "config.driver") } logger.Info("Connecting mail client...", zap.String("driver", config.Driver)) if config.Driver == "dummy" { @@ -45,7 +45,7 @@ func createMailClient(logger mlogger.Logger, producer messaging.Producer, l loca return mi.NewClient(logger, l, dp, &gsmconfing), nil } - return nil, merrors.InvalidArgument("Unkwnown mail driver: " + config.Driver) + return nil, merrors.InvalidArgument("Unkwnown mail driver: "+config.Driver, "config.driver") } func CreateMailClient(logger mlogger.Logger, sender string, producer messaging.Producer, l localizer.Localizer, dp domainprovider.DomainProvider, config *Config) (Client, error) { diff --git a/api/notification/internal/server/notificationimp/notification.go b/api/notification/internal/server/notificationimp/notification.go index c5d32c3..b6a752e 100644 --- a/api/notification/internal/server/notificationimp/notification.go +++ b/api/notification/internal/server/notificationimp/notification.go @@ -39,10 +39,10 @@ func CreateAPI(a api.API) (*NotificationAPI, error) { p.logger = a.Logger().Named(p.Name()) if a.Config().Notification == nil { - return nil, merrors.InvalidArgument("notification configuration is missing") + return nil, merrors.InvalidArgument("notification configuration is missing", "config.notification") } if a.Config().Notification.Telegram == nil { - return nil, merrors.InvalidArgument("telegram configuration is missing") + return nil, merrors.InvalidArgument("telegram configuration is missing", "config.notification.telegram") } var err error diff --git a/api/notification/internal/server/notificationimp/telegram/client.go b/api/notification/internal/server/notificationimp/telegram/client.go index 07356bc..fda0c4c 100644 --- a/api/notification/internal/server/notificationimp/telegram/client.go +++ b/api/notification/internal/server/notificationimp/telegram/client.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "fmt" + "html" "io" "net/http" "os" @@ -47,15 +48,15 @@ type sendMessagePayload struct { func NewClient(logger mlogger.Logger, cfg *notconfig.TelegramConfig) (Client, error) { if cfg == nil { - return nil, merrors.InvalidArgument("telegram configuration is not provided") + return nil, merrors.InvalidArgument("telegram configuration is not provided", "config.notification.telegram") } token := strings.TrimSpace(os.Getenv(cfg.BotTokenEnv)) if token == "" { - return nil, merrors.InvalidArgument(fmt.Sprintf("telegram bot token env %s is empty", cfg.BotTokenEnv)) + return nil, merrors.InvalidArgument(fmt.Sprintf("telegram bot token env %s is empty", cfg.BotTokenEnv), cfg.BotTokenEnv) } chatID := strings.TrimSpace(os.Getenv(cfg.ChatIDEnv)) if chatID == "" { - return nil, merrors.InvalidArgument(fmt.Sprintf("telegram chat id env %s is empty", cfg.ChatIDEnv)) + return nil, merrors.InvalidArgument(fmt.Sprintf("telegram chat id env %s is empty", cfg.ChatIDEnv), cfg.ChatIDEnv) } var threadID *int64 @@ -64,7 +65,7 @@ func NewClient(logger mlogger.Logger, cfg *notconfig.TelegramConfig) (Client, er if raw != "" { val, err := strconv.ParseInt(raw, 10, 64) if err != nil { - return nil, merrors.InvalidArgumentWrap(err, fmt.Sprintf("telegram thread id env %s is invalid", env)) + return nil, merrors.InvalidArgumentWrap(err, fmt.Sprintf("telegram thread id env %s is invalid", env), env) } threadID = &val } @@ -79,6 +80,10 @@ func NewClient(logger mlogger.Logger, cfg *notconfig.TelegramConfig) (Client, er if apiURL == "" { apiURL = defaultAPIURL } + parseMode := strings.TrimSpace(cfg.ParseMode) + if parseMode == "" { + parseMode = "Markdown" + } return &client{ logger: logger.Named("telegram"), @@ -89,15 +94,15 @@ func NewClient(logger mlogger.Logger, cfg *notconfig.TelegramConfig) (Client, er botToken: token, chatID: chatID, threadID: threadID, - parseMode: strings.TrimSpace(cfg.ParseMode), + parseMode: parseMode, }, nil } func (c *client) SendDemoRequest(ctx context.Context, request *model.DemoRequest) error { if request == nil { - return merrors.InvalidArgument("demo request payload is nil") + return merrors.InvalidArgument("demo request payload is nil", "request") } - message := buildMessage(request) + message := buildMessage(request, c.parseMode) payload := sendMessagePayload{ ChatID: c.chatID, Text: message, @@ -157,16 +162,89 @@ func (c *client) endpoint() string { return fmt.Sprintf("%s/bot%s/sendMessage", c.apiURL, c.botToken) } -func buildMessage(req *model.DemoRequest) string { +func buildMessage(req *model.DemoRequest, parseMode string) string { var builder strings.Builder builder.WriteString("New demo request received\n") - builder.WriteString(fmt.Sprintf("Name: %s\n", req.Name)) - builder.WriteString(fmt.Sprintf("Organization: %s\n", req.OrganizationName)) - builder.WriteString(fmt.Sprintf("Phone: %s\n", req.Phone)) - builder.WriteString(fmt.Sprintf("Work email: %s\n", req.WorkEmail)) - builder.WriteString(fmt.Sprintf("Payout volume: %s\n", req.PayoutVolume)) - if req.Comment != "" { - builder.WriteString(fmt.Sprintf("Comment: %s\n", req.Comment)) + builder.WriteString("-----------------------------\n") + + formatter := selectValueFormatter(parseMode) + appendMessageField(&builder, "Name", req.Name, formatter) + appendMessageField(&builder, "Organization", req.OrganizationName, formatter) + appendMessageField(&builder, "Phone", req.Phone, formatter) + appendMessageField(&builder, "Work email", req.WorkEmail, formatter) + appendMessageField(&builder, "Payout volume", req.PayoutVolume, formatter) + if strings.TrimSpace(req.Comment) != "" { + appendMessageField(&builder, "Comment", req.Comment, formatter) } return builder.String() } + +type valueFormatter func(string) string + +func appendMessageField(builder *strings.Builder, label, value string, formatter valueFormatter) { + value = strings.TrimSpace(value) + if value == "" { + value = "—" + } else if formatter != nil { + value = formatter(value) + } + fmt.Fprintf(builder, "• %s: %s\n", label, value) +} + +func selectValueFormatter(parseMode string) valueFormatter { + switch strings.ToLower(parseMode) { + case "markdown": + return func(value string) string { + return fmt.Sprintf("*%s*", escapeMarkdown(value)) + } + case "markdownv2": + return func(value string) string { + return fmt.Sprintf("*%s*", escapeMarkdownV2(value)) + } + case "html": + return func(value string) string { + return fmt.Sprintf("%s", html.EscapeString(value)) + } + default: + return nil + } +} + +var markdownEscaper = strings.NewReplacer( + "*", "\\*", + "_", "\\_", + "[", "\\[", + "]", "\\]", + "(", "\\(", + ")", "\\)", + "`", "\\`", +) + +var markdownV2Escaper = strings.NewReplacer( + "_", "\\_", + "*", "\\*", + "[", "\\[", + "]", "\\]", + "(", "\\(", + ")", "\\)", + "~", "\\~", + "`", "\\`", + ">", "\\>", + "#", "\\#", + "+", "\\+", + "-", "\\-", + "=", "\\=", + "|", "\\|", + "{", "\\{", + "}", "\\}", + ".", "\\.", + "!", "\\!", +) + +func escapeMarkdown(value string) string { + return markdownEscaper.Replace(value) +} + +func escapeMarkdownV2(value string) string { + return markdownV2Escaper.Replace(value) +} diff --git a/api/pkg/auth/factory.go b/api/pkg/auth/factory.go index b4b6a55..c054e56 100644 --- a/api/pkg/auth/factory.go +++ b/api/pkg/auth/factory.go @@ -48,5 +48,5 @@ func CreateAuth( } return enforcer, manager, nil } - return nil, nil, merrors.InvalidArgument("Unknown enforcer type: " + string(config.Driver)) + return nil, nil, merrors.InvalidArgument("Unknown enforcer type: "+string(config.Driver), "config.driver") } diff --git a/api/pkg/auth/internal/casbin/action.go b/api/pkg/auth/internal/casbin/action.go index 8e25dad..e19efb1 100644 --- a/api/pkg/auth/internal/casbin/action.go +++ b/api/pkg/auth/internal/casbin/action.go @@ -18,6 +18,6 @@ func stringToAction(actionStr string) (model.Action, error) { case string(model.ActionDelete): return model.ActionDelete, nil default: - return "", merrors.InvalidArgument(fmt.Sprintf("invalid action: %s", actionStr)) + return "", merrors.InvalidArgument(fmt.Sprintf("invalid action: %s", actionStr), "action") } } diff --git a/api/pkg/auth/internal/casbin/config/config.go b/api/pkg/auth/internal/casbin/config/config.go index 17d1f47..4984fef 100644 --- a/api/pkg/auth/internal/casbin/config/config.go +++ b/api/pkg/auth/internal/casbin/config/config.go @@ -109,7 +109,7 @@ func PrepareConfig(logger mlogger.Logger, config *Config) (*EnforcerConfig, erro if len(adapter.DatabaseName) == 0 { logger.Error("Database name is not set") - return nil, merrors.InvalidArgument("database name must be provided") + return nil, merrors.InvalidArgument("database name must be provided", "adapter.databaseName") } path := getEnvValue(logger, "model_path", "model_path_env", config.ModelPath, config.ModelPathEnv) diff --git a/api/pkg/auth/internal/casbin/role.go b/api/pkg/auth/internal/casbin/role.go index cc42979..7c0811e 100644 --- a/api/pkg/auth/internal/casbin/role.go +++ b/api/pkg/auth/internal/casbin/role.go @@ -35,7 +35,7 @@ func NewRoleManager(logger mlogger.Logger, enforcer *CasbinEnforcer, rolePermiss func (rm *RoleManager) validateObjectIDs(ids ...primitive.ObjectID) error { for _, id := range ids { if id.IsZero() { - return merrors.InvalidArgument("Object references cannot be zero") + return merrors.InvalidArgument("Object references cannot be zero", "objectRef") } } return nil diff --git a/api/pkg/auth/internal/native/role.go b/api/pkg/auth/internal/native/role.go index 2515b2c..f50eaa6 100644 --- a/api/pkg/auth/internal/native/role.go +++ b/api/pkg/auth/internal/native/role.go @@ -36,7 +36,7 @@ func NewRoleManager(logger mlogger.Logger, enforcer *Enforcer, rolePermissionRef func (rm *RoleManager) validateObjectIDs(ids ...primitive.ObjectID) error { for _, id := range ids { if id.IsZero() { - return merrors.InvalidArgument("Object references cannot be zero") + return merrors.InvalidArgument("Object references cannot be zero", "objectRef") } } return nil diff --git a/api/pkg/db/connection.go b/api/pkg/db/connection.go index da45fca..de00fbf 100644 --- a/api/pkg/db/connection.go +++ b/api/pkg/db/connection.go @@ -47,10 +47,10 @@ func (c *MongoConnection) Ping(ctx context.Context) error { // ConnectMongo returns a low-level MongoDB connection without constructing repositories. func ConnectMongo(logger mlogger.Logger, config *Config) (*MongoConnection, error) { if config == nil { - return nil, merrors.InvalidArgument("database configuration is nil") + return nil, merrors.InvalidArgument("database configuration is nil", "config") } if config.Driver != Mongo { - return nil, merrors.InvalidArgument("unsupported database driver: " + string(config.Driver)) + return nil, merrors.InvalidArgument("unsupported database driver: "+string(config.Driver), "config.driver") } client, _, settings, err := mongoimpl.ConnectClient(logger, config.Settings) diff --git a/api/pkg/db/factory.go b/api/pkg/db/factory.go index f43697d..dc8c7e8 100644 --- a/api/pkg/db/factory.go +++ b/api/pkg/db/factory.go @@ -37,5 +37,5 @@ func NewConnection(logger mlogger.Logger, config *Config) (Factory, error) { if config.Driver == Mongo { return mongoimpl.NewConnection(logger, config.Settings) } - return nil, merrors.InvalidArgument("unknown database driver: " + string(config.Driver)) + return nil, merrors.InvalidArgument("unknown database driver: "+string(config.Driver), "config.driver") } diff --git a/api/pkg/db/internal/mongo/organizationdb/create.go b/api/pkg/db/internal/mongo/organizationdb/create.go index 9f69ae4..2be8cd2 100644 --- a/api/pkg/db/internal/mongo/organizationdb/create.go +++ b/api/pkg/db/internal/mongo/organizationdb/create.go @@ -10,7 +10,7 @@ import ( func (db *OrganizationDB) Create(ctx context.Context, _, _ primitive.ObjectID, org *model.Organization) error { if org == nil { - return merrors.InvalidArgument("Organization object is nil") + return merrors.InvalidArgument("Organization object is nil", "organization") } org.SetID(primitive.NewObjectID()) // Organizaiton reference must be set to the same value as own organization reference diff --git a/api/pkg/db/internal/mongo/refreshtokensdb/crud.go b/api/pkg/db/internal/mongo/refreshtokensdb/crud.go index 7365b88..92699f3 100644 --- a/api/pkg/db/internal/mongo/refreshtokensdb/crud.go +++ b/api/pkg/db/internal/mongo/refreshtokensdb/crud.go @@ -18,7 +18,7 @@ func (db *RefreshTokenDB) Create(ctx context.Context, rt *model.RefreshToken) er // First, try to find an existing token for this account/client/device combination var existing model.RefreshToken if rt.AccountRef == nil { - return merrors.InvalidArgument("Account reference must have a vaild value") + return merrors.InvalidArgument("Account reference must have a vaild value", "refreshToken.accountRef") } if err := db.FindOne(ctx, filterByAccount(*rt.AccountRef, &rt.SessionIdentifier), &existing); err != nil { if errors.Is(err, merrors.ErrNoData) { diff --git a/api/pkg/db/internal/mongo/repositoryimp/index.go b/api/pkg/db/internal/mongo/repositoryimp/index.go index 9a3a31b..7313aa7 100644 --- a/api/pkg/db/internal/mongo/repositoryimp/index.go +++ b/api/pkg/db/internal/mongo/repositoryimp/index.go @@ -15,7 +15,7 @@ func (r *MongoRepository) CreateIndex(def *ri.Definition) error { return merrors.NoData("data collection is not set") } if len(def.Keys) == 0 { - return merrors.InvalidArgument("Index definition has no keys") + return merrors.InvalidArgument("Index definition has no keys", "index.keys") } // ----- build BSON keys -------------------------------------------------- diff --git a/api/pkg/db/internal/mongo/repositoryimp/repository.go b/api/pkg/db/internal/mongo/repositoryimp/repository.go index adac397..81e2cc4 100644 --- a/api/pkg/db/internal/mongo/repositoryimp/repository.go +++ b/api/pkg/db/internal/mongo/repositoryimp/repository.go @@ -83,7 +83,7 @@ func (r *MongoRepository) findOneByFilterImp(ctx context.Context, filter bson.D, func (r *MongoRepository) Get(ctx context.Context, id primitive.ObjectID, result storable.Storable) error { if id.IsZero() { - return merrors.InvalidArgument("zero id provided while fetching " + result.Collection()) + return merrors.InvalidArgument("zero id provided while fetching "+result.Collection(), "id") } return r.findOneByFilterImp(ctx, idFilter(id), fmt.Sprintf("%s with ID = %s not found", result.Collection(), id.Hex()), result) } @@ -134,7 +134,7 @@ func (r *MongoRepository) Update(ctx context.Context, obj storable.Storable) err func (r *MongoRepository) Patch(ctx context.Context, id primitive.ObjectID, patch builder.Patch) error { if id.IsZero() { - return merrors.InvalidArgument("zero id provided while patching") + return merrors.InvalidArgument("zero id provided while patching", "id") } _, err := r.collection.UpdateByID(ctx, id, patch.Build()) return err diff --git a/api/pkg/db/internal/mongo/tseriesimp/tseries.go b/api/pkg/db/internal/mongo/tseriesimp/tseries.go index f7713c0..ed1a5ba 100644 --- a/api/pkg/db/internal/mongo/tseriesimp/tseries.go +++ b/api/pkg/db/internal/mongo/tseriesimp/tseries.go @@ -22,7 +22,7 @@ type TimeSeries struct { func NewMongoTimeSeriesCollection(ctx context.Context, db *mongo.Database, tsOpts *tsoptions.Options) (*TimeSeries, error) { if tsOpts == nil { - return nil, merrors.InvalidArgument("nil time-series options provided") + return nil, merrors.InvalidArgument("nil time-series options provided", "options") } // Configure time-series options granularity := tsOpts.Granularity.String() diff --git a/api/pkg/merrors/errors.go b/api/pkg/merrors/errors.go index 3667374..754db56 100644 --- a/api/pkg/merrors/errors.go +++ b/api/pkg/merrors/errors.go @@ -3,6 +3,7 @@ package merrors import ( "errors" "fmt" + "strings" "go.mongodb.org/mongo-driver/bson/primitive" ) @@ -27,8 +28,8 @@ func Internal(msg string) error { var ErrInvalidArg = errors.New("invalidArgError") -func InvalidArgument(msg string) error { - return fmt.Errorf("%w: %s", ErrInvalidArg, msg) +func InvalidArgument(msg string, argumentNames ...string) error { + return fmt.Errorf("%w: %s", ErrInvalidArg, invalidArgumentMessage(msg, argumentNames...)) } var ErrDataConflict = errors.New("DataConflict") @@ -64,8 +65,8 @@ func NoMessagingTopic(topic string) error { return fmt.Errorf("%w: messaging topic '%s' not found", ErrNoMessagingTopic, topic) } -func InvalidArgumentWrap(err error, msg string) error { - return wrapError(ErrInvalidArg, msg, err) +func InvalidArgumentWrap(err error, msg string, argumentNames ...string) error { + return wrapError(ErrInvalidArg, invalidArgumentMessage(msg, argumentNames...), err) } func InternalWrap(err error, msg string) error { @@ -79,3 +80,23 @@ func wrapError(base error, msg string, err error) error { } return errors.Join(baseErr, err) } + +func invalidArgumentMessage(msg string, argumentNames ...string) string { + names := make([]string, 0, len(argumentNames)) + for _, name := range argumentNames { + name = strings.TrimSpace(name) + if name == "" { + continue + } + names = append(names, fmt.Sprintf("%q", name)) + } + if len(names) == 0 { + return msg + } + + prefix := "broken argument" + if len(names) > 1 { + prefix = "broken arguments" + } + return fmt.Sprintf("%s %s: %s", prefix, strings.Join(names, ", "), msg) +} diff --git a/api/pkg/merrors/errors_test.go b/api/pkg/merrors/errors_test.go new file mode 100644 index 0000000..bb6cedd --- /dev/null +++ b/api/pkg/merrors/errors_test.go @@ -0,0 +1,47 @@ +package merrors + +import ( + "errors" + "strings" + "testing" +) + +func TestInvalidArgumentSupportsBrokenArgumentName(t *testing.T) { + t.Run("without argument name keeps old behavior", func(t *testing.T) { + err := InvalidArgument("value is missing") + expected := "invalidArgError: value is missing" + if err.Error() != expected { + t.Fatalf("unexpected error message: %s", err) + } + if !errors.Is(err, ErrInvalidArg) { + t.Fatalf("error should wrap ErrInvalidArg") + } + }) + + t.Run("single argument name", func(t *testing.T) { + err := InvalidArgument("value is missing", "bot_token_env") + expected := `invalidArgError: broken argument "bot_token_env": value is missing` + if err.Error() != expected { + t.Fatalf("unexpected error message: %s", err) + } + }) + + t.Run("multiple argument names", func(t *testing.T) { + err := InvalidArgument("value mismatch", "bot_token_env", "chat_id_env", " ") + expected := `invalidArgError: broken arguments "bot_token_env", "chat_id_env": value mismatch` + if err.Error() != expected { + t.Fatalf("unexpected error message: %s", err) + } + }) +} + +func TestInvalidArgumentWrapSupportsBrokenArgumentName(t *testing.T) { + base := errors.New("root cause") + err := InvalidArgumentWrap(base, "value is missing", "bot_token_env") + if !strings.Contains(err.Error(), `invalidArgError: broken argument "bot_token_env": value is missing`) { + t.Fatalf("wrapped error should include broken argument name: %s", err) + } + if !errors.Is(err, ErrInvalidArg) || !errors.Is(err, base) { + t.Fatalf("wrapped error should preserve all error layers") + } +} diff --git a/api/pkg/messaging/internal/inprocess/broker.go b/api/pkg/messaging/internal/inprocess/broker.go index dc54b20..3be97aa 100644 --- a/api/pkg/messaging/internal/inprocess/broker.go +++ b/api/pkg/messaging/internal/inprocess/broker.go @@ -76,7 +76,7 @@ func (b *MessageBroker) Unsubscribe(event model.NotificationEvent, subChan <-cha func NewInProcessBroker(logger mlogger.Logger, bufferSize int) (*MessageBroker, error) { if bufferSize < 1 { - return nil, merrors.InvalidArgument(fmt.Sprintf("Invelid buffer size %d. It must be greater than 1", bufferSize)) + return nil, merrors.InvalidArgument(fmt.Sprintf("Invelid buffer size %d. It must be greater than 1", bufferSize), "bufferSize") } logger.Info("Created in-process logger", zap.Int("buffer_size", bufferSize)) return &MessageBroker{ diff --git a/api/pkg/messaging/internal/natsb/broker.go b/api/pkg/messaging/internal/natsb/broker.go index 14cb51c..b2e2b97 100644 --- a/api/pkg/messaging/internal/natsb/broker.go +++ b/api/pkg/messaging/internal/natsb/broker.go @@ -39,7 +39,7 @@ func loadEnv(settings *nc.Settings, l *zap.Logger) (*envConfig, error) { return v, nil } l.Error(fmt.Sprintf("NATS %s not found in environment", label), zap.String("env_var", key)) - return "", merrors.InvalidArgument(fmt.Sprintf("NATS %s not found in environment variable: %s", label, key)) + return "", merrors.InvalidArgument(fmt.Sprintf("NATS %s not found in environment variable: %s", label, key), key) } user, err := get(settings.UsernameEnv, "user name") @@ -65,7 +65,7 @@ func loadEnv(settings *nc.Settings, l *zap.Logger) (*envConfig, error) { port, err := strconv.Atoi(portStr) if err != nil || port <= 0 || port > 65535 { l.Error("Invalid NATS port value", zap.String("port", portStr)) - return nil, merrors.InvalidArgument("Invalid NATS port: " + portStr) + return nil, merrors.InvalidArgument("Invalid NATS port: "+portStr, settings.PortEnv) } return &envConfig{ diff --git a/api/pkg/messaging/internal/notifications/site/notification.go b/api/pkg/messaging/internal/notifications/site/notification.go index d40f824..c0b92ec 100644 --- a/api/pkg/messaging/internal/notifications/site/notification.go +++ b/api/pkg/messaging/internal/notifications/site/notification.go @@ -17,7 +17,7 @@ type DemoRequestNotification struct { func (drn *DemoRequestNotification) Serialize() ([]byte, error) { if drn.request == nil { - return nil, merrors.InvalidArgument("demo request payload is empty") + return nil, merrors.InvalidArgument("demo request payload is empty", "request") } msg := gmessaging.DemoRequestEvent{ Name: drn.request.Name, diff --git a/api/pkg/model/demorequest.go b/api/pkg/model/demorequest.go index 007b357..c7c7e54 100644 --- a/api/pkg/model/demorequest.go +++ b/api/pkg/model/demorequest.go @@ -32,22 +32,22 @@ func (dr *DemoRequest) Normalize() { // Validate ensures that all required fields are present. func (dr *DemoRequest) Validate() error { if dr == nil { - return merrors.InvalidArgument("request payload is empty") + return merrors.InvalidArgument("request payload is empty", "request") } if dr.Name == "" { - return merrors.InvalidArgument("name must not be empty") + return merrors.InvalidArgument("name must not be empty", "request.name") } if dr.OrganizationName == "" { - return merrors.InvalidArgument("organization name must not be empty") + return merrors.InvalidArgument("organization name must not be empty", "request.organizationName") } if dr.Phone == "" { - return merrors.InvalidArgument("phone must not be empty") + return merrors.InvalidArgument("phone must not be empty", "request.phone") } if dr.WorkEmail == "" { - return merrors.InvalidArgument("work email must not be empty") + return merrors.InvalidArgument("work email must not be empty", "request.workEmail") } if dr.PayoutVolume == "" { - return merrors.InvalidArgument("payout volume must not be empty") + return merrors.InvalidArgument("payout volume must not be empty", "request.payoutVolume") } return nil } diff --git a/api/pkg/mutil/reorder/reorder.go b/api/pkg/mutil/reorder/reorder.go index 891e797..14f02d6 100644 --- a/api/pkg/mutil/reorder/reorder.go +++ b/api/pkg/mutil/reorder/reorder.go @@ -17,12 +17,12 @@ func IndexableRefs(items []model.IndexableRef, oldIndex, newIndex int) ([]model. } } if targetIndex == -1 { - return nil, merrors.InvalidArgument("Item not found at specified index") + return nil, merrors.InvalidArgument("Item not found at specified index", "oldIndex") } // Validate new index bounds if newIndex < 0 || newIndex >= len(items) { - return nil, merrors.InvalidArgument("Invalid new index for reorder") + return nil, merrors.InvalidArgument("Invalid new index for reorder", "newIndex") } // Remove the item from its current position diff --git a/api/pkg/server/grpcapp/app.go b/api/pkg/server/grpcapp/app.go index d2d0b6d..8a35d17 100644 --- a/api/pkg/server/grpcapp/app.go +++ b/api/pkg/server/grpcapp/app.go @@ -53,13 +53,13 @@ type App[T any] struct { func NewApp[T any](logger mlogger.Logger, name string, config *Config, debug bool, repoFactory RepositoryFactory[T], serviceFactory ServiceFactory[T], opts ...Option[T]) (*App[T], error) { if logger == nil { - return nil, merrors.InvalidArgument("nil logger supplied") + return nil, merrors.InvalidArgument("nil logger supplied", "logger") } if config == nil { - return nil, merrors.InvalidArgument("nil config supplied") + return nil, merrors.InvalidArgument("nil config supplied", "config") } if serviceFactory == nil { - return nil, merrors.InvalidArgument("nil service factory supplied") + return nil, merrors.InvalidArgument("nil service factory supplied", "serviceFactory") } app := &App[T]{