Skip to content
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

Autopaho managePublishQueue() does not call Remove, Quarantine, or Leave if Reader() fails #233

Closed
vishnureddy17 opened this issue Jan 29, 2024 · 4 comments · Fixed by #235

Comments

@vishnureddy17
Copy link
Contributor

In autopaho, if entry.Reader() fails we do not call Remove, Quarantine, or Leave on the entry as required. I'm guessing there should be a call to Leave() in the case where Reader() fails. Happy to open a PR for this.
https://github.com/eclipse/paho.golang/blob/ef0065fea247d5fb388fa82390cedce694e844e1/autopaho/auto.go#L553-L562

@MattBrittan
Copy link
Contributor

Well spotted; happy to accept a PR. I'm not not sure if it's best to leave or Remove in this case; if we cannot read the message then what are the chances that this would succeed if we try again.

@vishnureddy17
Copy link
Contributor Author

if we cannot read the message then what are the chances that this would succeed if we try again.

This is the case that prompted me to open #234

@MattBrittan
Copy link
Contributor

Just noticed a further bug here; we check if errors.Is(err, queue.ErrEmpty) { but ignore other errors (not sure what the right thing to do is; perhaps a small timeout then loop).

@vishnureddy17
Copy link
Contributor Author

(not sure what the right thing to do is; perhaps a small timeout then loop).

That seems reasonable. I also think we should have that same timeout in the default case if PublishWithOptions fails with an error that is not paho.ErrNetworkErrorAfterStored or paho.ErrInvalidArguments. Right now, we immediately loop without a delay:
https://github.com/eclipse/paho.golang/blob/ef0065fea247d5fb388fa82390cedce694e844e1/autopaho/auto.go#L612-L621

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants