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[bot]]

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!