FEAT: [coinbase] implement stream message handlers #1924
Conversation
| for _, subCmd := range subCmds { | ||
| err := s.Conn.WriteJSON(subCmd) | ||
| if err != nil { | ||
| log.WithError(err).Errorf("subscription error: %v", subCmd) |
There was a problem hiding this comment.
Long with %v prints all fields of the struct. The struct contains exchange credentials. We shouldn't log the credentials. Note that error logs may also be sent to 3 party systems such as rollbar or sentry. It leaks the credential.
There was a problem hiding this comment.
Fixed.
I also add a test on it.
| // query account balances | ||
| balances, err := s.exchange.QueryAccountBalances(ctx) | ||
| if err != nil { | ||
| log.WithError(err).Warn("failed to query account balances, the balance snapshot is initialized with empty balances") |
There was a problem hiding this comment.
Should we emit empty balance snapshot when an error is occurred?
There was a problem hiding this comment.
After consult with @c9s, we prefer to emit an empty balance.
Will fix it accordingly.
86bfeb2 to
1266af0
Compare
1266af0 to
1d69a90
Compare
| } | ||
|
|
||
| func (c channelType) String() string { | ||
| return fmt.Sprintf(`(Name: %s, |
There was a problem hiding this comment.
Looks like you add a tab before the value?
You can insert a tab using the \t directive, but in this case, I guess it's either a typo or you intend to align each line manually. However, when reading logs on different platforms such as Kibana, they may parse the logs differently, which can affect the alignment and make it look different from what you intended.
There was a problem hiding this comment.
Got it.
I will simplify the format string.
| Key: ****, | ||
| Passphrase: ****, |
There was a problem hiding this comment.
We don't have to print all fields of the struct, so we could just ignore key and passphrase
| func (msg subscribeMsgType1) String() string { | ||
| return fmt.Sprintf(`(Type: %s, | ||
| Channels: %s, | ||
| %s)`, msg.Type, msg.Channels, msg.authMsg.String()) | ||
| } |
…s and enhance connection logic
…ging and prevent leaking the credentials
f34d3cc to
63ba0bd
Compare
63ba0bd to
92946f1
Compare
In this PR: