Bug 1428 ib critical errors

[vishalg]

Amended IB connection method to catch all exceptions, log a critical error message and re-raise.
Changed ib_client.py error_handler to log all broker_errors as critical rather than as warnings.

[bug-or-feature]

@vishalg have you tested this? I'm a bit concerned about the change to ib_client.py - if a large bunch of critical errors happen in a short burst, triggering lots of emails, it CAN look like spam to an SMTP server. This bit me once before, my own mail server got blacklisted and it was a pain to unwind. Ideally we would have an email sending queue, or a system to throttle a high number of identical messages in a short time.

[vishalg]

I have not had an such an experience yet, but I imagine it can cause an issue. i'll revert that change but we should still log the IB connection errors as critical. This happens to me quite a lot - I run TWS in docker with 2FA enabled and sometimes miss the notification when TWS gets restarted on Sundays.

[vishalg]

I think we should change the PstSMTPHandler in syslogging.handlers to use email_via_db_interface rather than the send_msg directly. If you agree, I can make the changes and update

[bug-or-feature]

I think it would be better to replace PstSMTPHandler with one with a queue or buffer. I'm pretty sure I saw some code for one already. To be honest I should have done it at the time of the log rewrite. I'll do this, just let me test for a few days next week

[vishalg]

Yeah, that makes sense. Logging handlers don't currently have access to the data pipeline objects though - if the queue is held in memory messages can get lost and we'd need a way to persist to disk. I'll have a stab at it and submit a new request along with a change to the way SMTP connections are handled currently.

[bug-or-feature]

@vishalg Digging into this a bit more, I don't think the log records coming from ib_client.py should be critical. Many of the kind of messages that are supposed to be handled there are things like "price out of range", "modify order failed", "order rejected". Those are not critical situations. I do think they should be error rather than warning. Admittedly, "can't connect" is in there too - maybe there should be some modified logic to handle that better? I'm not sure, I don't know this code very well. But, if you're really sure you want an email for any of those types of problems, then I think a better solution would use a custom logging config

[vishalg]

That was my initial thoughts as well, but looking at IB_ERROR_TYPES (sysbrokers.IB.client.ib_client.py) a majority of them seem to be related to orders - although I am not sure to what extent these definitely need user attention.

There may be a case for augmenting IB_ERROR_TYPES with severity level and then deal with them accordingly.

I will park this for now until I have had more experience running the system on a live account and collected enough instances of when and how these messages are being generated.

[bug-or-feature]

Agreed, I'll do same. Here's the full list of messages from IB:

https://ibkrcampus.com/campus/ibkr-api-page/twsapi-doc/#error-handling