absmach / magistrala

Industrial IoT Messaging and Device Management Platform
https://www.abstractmachines.fr/magistrala.html
Apache License 2.0
2.42k stars 665 forks source link

Bug: Log doesn't have full errors, instead it returns limited error information #2260

Open arvindh123 opened 4 weeks ago

arvindh123 commented 4 weeks ago

What were you trying to achieve?

I'm trying to simulate on database rollback error for a service,

What are the expected results?

Logs with rollback error ( log with all nested errors)

What are the received results?

Logs without rollback error ( log with top two layer errors)

Steps To Reproduce

Modified source code to throw rollback error in any service for create entity

In what environment did you encounter the issue?

Docker

Additional information you deem important

We should replace args = append(args, slog.Any("error", err)) with args = append(args, slog.String("error", err.Error()))

Because slog.Any("error", err) use JSON Marshal to stringify the error and We have custom JSON Marshal for Magistrala errors https://github.com/absmach/magistrala/blob/main/pkg/errors/errors.go#L59-L71, So in logs we could not get full error .

func (ce *customError) MarshalJSON() ([]byte, error) {
    var val string
    if e := ce.Err(); e != nil {
        val = e.Msg()
    }
    return json.Marshal(&struct {
        Err string `json:"error"`
        Msg string `json:"message"`
    }{
        Err: val,
        Msg: ce.Msg(),
    })
}
rodneyosodo commented 3 weeks ago

If we use slog.String("error", err.Error() we will omit message from the logs. Is this ideal?

arvindh123 commented 3 weeks ago

Magistrala Error() returns error string including message. https://github.com/absmach/magistrala/blob/main/pkg/errors/errors.go#L41-L49 Is this correct ?