not all sync failures should trigger a disconnect

[caver456]

See comments starting with this one in the radiolog location markers issue.

• differentiate exceptions that should cause a disconnect from those that should not, and modify code accordingly
• fix the specific cause of [caltopo_python:1246:ERROR:syncThread] Exception in _doSync:list index out of range

[caver456]

Disconnect should only happen if there isn't a valid response:

• response code other than 200
• timeout
• ('Connection aborted.', RemoteDisconnected('Remote end closed connection without response') or similar

Otherwise, it's not a disconnect, even if there is some exception during processing of that valid response.

[caver456]

Another key note, just added to the radiolog issue comment:

All of the 'list index out of range' lines (the illegit disconnects) had the following line:

[caltopo_python:1419:INFO:syncThread] disconnected (first exception during response from sync); queue size is 0

And all of the legit disconnects had the following line:

[caltopo_python:1242:INFO:syncThread] disconnected (first failed response from sync); queue size is 0

[caver456]

Line 1419 (the illegit disconnects) is in _syncLoop, but, the exception was raised from a handled exception in _doSync line 1248, just below the 1242 disconnect:

        elif self.latestResponseCode in [401]:
            logging.error('Sync attempt returned '+str(self.latestResponseCode))
            self.closeMap()
            if self.mapClosedCallback:
                self.mapClosedCallback(self.badResponse)
        else:
            logging.error('Sync returned invalid or no response; sync aborted:'+str(rj))
            # self.sync=False
            # self.apiVersion=-1 # downstream tools may use apiVersion as indicator of link status
            # logging.error('Sync attempt failed; setting holdRequests')
            logging.error('Sync attempt failed')
            # self.holdRequests=True
            if not self.disconnectedFlag:
                self._disconnectedFlagSet()
1242:           logging.info('disconnected (first failed response from sync); queue size is '+str(self.requestQueue.qsize()))
                if self.disconnectedCallback:
                    self.disconnectedCallback()
    except Exception as e:
        logging.error('Exception in _doSync:'+str(e))
        if fromLoop:
1248:       raise e # let _syncLoop handle it, which will cause a disconnect condition

So, if we just don't raise the exception on 1248, then the illegit disconnects, from this particular cause, wouldn't happen. But, is that the right thing to do?

[caver456]

Using this code shows the line number within the same method that causes the exception:

        except Exception as e:
            logging.error(f'Exception in _doSync line {sys.exc_info()[2].tb_lineno}: {e}')

So if the exception is one level down, within a function called from the same function with the above lines, then it will only print the line of that function call - it won't print the line within the called function's code. Better than nothing...

[caver456]

So - should a single exception (i.e. buggy code) trigger a disconnect? Philosophical question. Maybe best to have session variables/arguments to determine how many consecutive sync exceptions should trigger a disconnect, and separately, how many consecutive failed-sync-but-not-exceptions should trigger a disconnect. The former can just be a local variable in _syncLoop since _doSync exceptions are raised to _syncLoop where they can be counted, but the latter will need to be tracked as a session variable, since it needs to persist from one sync call to the next. There are other ways to handle it... if exceptions shouldn't be raised to _syncLoop then they could also be counted as a session variable from inside _doSync.

But it's also still important to find and squash the 'list index out of range' bug since it's the only observed exception. Can't reproduce it at home, either on a different test map, or on the map from the search in question. It's probably something about the map data at the time. Really looks like it's from the 'if idsBefore' clause.