Callback for error messages

[jnidzwetzki]

At the moment, all channel related errors are only logged in AccountInfoHandler:111. There should be a callback to receive all errors as a string or a JSON message..

[mironbalcerzak]

I agree with "callback" - however string/json message does not seem right.

I think we should raise an ApplicationException and the said callback would accept this exception. User may listen for that event.

What do you think?

[mironbalcerzak]

Ohh - it's related to gitter chat.
Well - scenario described there is not an application error, but user action error.

Information is being received to "notification handler". I am not sure if we expose those events to user.
Well - we still could raise an exception in case "error" - then exception would be caught and passed to "onExceptionCallback" (not sure if it's good approach, slightly misleading).

...or - simply - we can just expose onNotificationEvent() callback with 2 arguments (NotificationType which is enum: UserError, ServerInfo, etc.etc) and 2nd arg as notification that we received - I like this approach better.

From what I see - it's slightly misleading, in AccountInfoHandler method is named onOrderNotification(), should be renamed to onNotification(), and we could parse the notifications there (extract type of notification)

-- mb

[jnidzwetzki]

At the moment we do:

if (message.toString().contains("ERROR")) { logger.error("Got error message: {}", message.toString()); }

to detect error messages. I could not find a generic format for error messages in the documentation. So, I'm not sure how to handle/parse these messages. We could put the messages in the exception as a string, but then the user still has to take care of the parsing.

I also like the onNotificationEvent() idea, but I am not sure what we should pass as the second argument to the user.

[mironbalcerzak]

Ohh - there is already a method exposed: (should be renamed to onMyNotification maybe? -- "my" is related to current AccountInfo - not general)
BitfinexApiCallbackListeners.onMyOrderNotification()

from what I see - we currently provide 2 args - (account), (order) - which is wrong in current scenario.
We can get from notification "error" string, as you said which gives us one type already.

I cannot check the documentation atm, will do in the evening and come back with proper possible solution.

Talk back soon
-- mb

[rubenfanjulestrada]

I am actually having problems with this issue, I can not find the way to get the info coming though AccountInfoHandler and actually ( did not see other solution ) looks like the only way to get the error, with clientId and error description.

I also think that onNotificationEvent could help is this type of cases, could be ok if I push a fix for that ?

[mironbalcerzak]

What exactly are you trying to get an event of? Account info related consists of:

Position snapshot
Position new
Position updated
Position canceled
Founding offer snapshot
Founding offer notification
Founding offer update
Founding offer cancel
Founding credit snapshot
Founding credit notification
Founding credit update
Founding credit cancel
Founding loans snapshot
Founding loans notification
Founding loans update
Founding loans cancel
Wallet snapshot
Wallet update
Order snapshot
Order notification
Order update
Order cancellation
Trade executed
Trade updates
General notification
Most of which, are already exposed in BitfinexApiCallbackListeners.

What are you missing? Your solution will cause spamming (bad) and duplication (bad) of all types of events on "account info" in "raw/untyped" (=java.util.String) manner (very bad).

EDIT:
What I suggest is, since we are handling "notifications" in NotificationHandler, move the logic there - and expose handler (the way you did for account info), but to notification handler. You need to do one more level of event propagation. And naturally, revert the changes you did :) thanks

EDIT2:
Provide some junits for the stuff you write too 👍

[github-actions]

This issue has been automatically marked as stale due to lack of activity. You can remove the stale label or comment. Otherwise, this issue will be closed in 7 days. Thank you!