Public receive method in SBPFramer Java class

[IsakTjernberg]

Need this method as public to be able to see if the connection is broken or not. Access outside package was only available through an iterator in SBPHandler which swallows the exception.

[jbangelo]

Seems reasonable to me, but I'm less versed on Java. @silverjam think this is a problem?

[silverjam]

Why is this needed? Can we add more description to the PR?

[IsakTjernberg]

@silverjam I updated the description with some more details.

[silverjam]

@IsakTjernberg @jbangelo Could we expose this state in the iterator instead? E.g. store the last exception that occurs, or record a flag and expose a new interface for that?

[IsakTjernberg]

@silverjam well, wouldn't that complicate things unnecessarily? The sendMessage() method is already public.
How would someone with a SBPHandler while looping over the iterator check for that exception/flag?

[jbangelo]

Can Java Iterable objects not throw exceptions from their getNext() implementations?

[silverjam]

@IsakTjernberg I’m thinking we should maintain the existing abstractions if possible, how does making this public fix the problem? Don’t you need to create your own reader / iterator logic in order to detect the error?

I was thinking SBPHandler would just gain a lastException() method that would return null or the last exception so you could detect why the iterator broke.

Or, we could create a variant of SBPHandler that doesn’t swallow the error.

[IsakTjernberg]

@silverjam Ok, fair point. Does the iterator() in SBPHandler ever break though? What happens in ReceiveThread in SBPHandler if source throws a NoSuchElementException? Does the thread just end and then the SBPQueueIterator will be waiting forever for a new item to be put in its BlockingQueue?

[silverjam]

@IsakTjernberg SBPIterable.hasNext should return false in that case and terminate the iterator, I believe? The pattern we have in Python is for the iterator to record it's exception, then the termination of the threaded stuff propagates the exception when the iterator stops (so SBPQueueIterator.hasNext would need to look for an exception recorded by the source in ReceiveThread and propagate/throw it.

[silverjam]

For reference, were the threaded iterator records the IO exception:

• https://github.com/swift-nav/libsbp/blob/master/python/sbp/client/handler.py#L69

And the "propagation":

• https://github.com/swift-nav/libsbp/blob/master/python/sbp/client/handler.py#L332

[silverjam]

In other words, we had this exact problem with the original design of the Python sbp client library (with it swallowing exceptions, so you couldn't detect connection drops)-- and it looks like the Java sbp client library mirrors Python in terms of trying to put the blocking receive logic on a thread, so it's not surprising that it has the same issue.

[IsakTjernberg]

@silverjam Ok, thanks! I'll try to replicate it similarly in the Java parts.

[IsakTjernberg]

@silverjam The new commit stops the iterator from forever waiting for new messages, but unfortunately does not expose the underlying exception. In SBPHandler the source is abstracted as a Iterable<SBPMessage>, whose iterator can only throw NoSuchElementException. So it is difficult to get the exception back up the chain without changing the type of source, which I did not want to do.
I'm happy to keep discussing if you see a better way.

[silverjam]

Ok, maybe we can tackle the exception bubbling later

One suggest on naming, otherwise looks fine

[silverjam]

Just “callbackDone” seems sufficient