-
Notifications
You must be signed in to change notification settings - Fork 95
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Incorrect assumption about PUBLISH errors in autopaho can cause indefinite head-of-queue looping #214
Comments
Agreed; I did consider doing something similar when introducing this, but felt that it was OK for an initial PR (hard to know when to stop and submit the PR). My thought was something like |
That sounds good, I also think it's worth returning such an error from Subscribe and Unsubscribe, which would be used to distinguish between errors that are due to a client failure/disconnection and errors that are due to something fatally wrong with the subscribe/unsubscribe |
I've had an initial go at this; went with your solution (it's probably useful to be able to differentiate between |
closing due to #226, @MattBrittan feel free to reopen if you still think it's important to keep around |
This is really only doable if #216 is addressed. Also, even it was possible, I don't think that |
In
autopaho
,managePublishQueue
assumes that ifpaho.PublishWithOptions()
returns an error that is notpaho.ErrNetworkErrorAfterStored
, the error is either temporary or the connection will drop:https://github.com/eclipse/paho.golang/blob/a6def521ee1aecf0b1cef6bfc1a7022102457807/autopaho/auto.go#L531-L553
However, this is not always the case. All the errors returned in the code snippet below for
paho.PublishWithOptions()
are neither temporary nor imply a pending disconnection:https://github.com/eclipse/paho.golang/blob/a6def521ee1aecf0b1cef6bfc1a7022102457807/paho/client.go#L748-L762
As a result,
managePublishQueue
will loop indefinitely on the first entry in the queue in these cases.Proposed Solution
To solve this, I propose creating a custom
error
type calledPahoArgumentError
, which will be returned bypaho.PublishWithOptions()
in these cases (thiserror
type will likely be relevant elsewhere inpaho
). Thenautopaho
can do a type assertion and quarantine any publish in the queue that returnedPahoArgumentError
.The text was updated successfully, but these errors were encountered: