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

Add a Roster callback for error Presences. #62

Closed

Conversation

damencho
Copy link
Contributor

@damencho damencho commented Dec 6, 2015

No description provided.

@damencho
Copy link
Contributor Author

damencho commented Dec 6, 2015

I've screwed up my git and created new PR.
Does this looks good?
One thing that bothers me is that in smack3.2 we were receiving a roster item that wasn't good and we were ignoring it. While here if an exception occurs roaster becomes uninitialized (rosterState = RosterState.uninitialized;). Anyway it is good to have roster errors notified to a listener with the packet holding the error.

@Flowdalic
Copy link
Member

I've screwed up my git and created new PR.

Note that you don't need to create new PRs. You could have just updated the existing PR by force pushing to its source branch.

Does this looks good?
One thing that bothers me is that in smack3.2 we were receiving a roster item that wasn't good and we were ignoring it. While here if an exception occurs roaster becomes uninitialized (rosterState = RosterState.uninitialized;). Anyway it is good to have roster errors notified to a listener with the packet holding the error.

I agree that a callback is missing if there is an error when initializing the Roster. What I don't like about 7b65040 is that it changes the Exception callback adding a Stanza parameter. But not always is a Stanza involved in the failure. Instead I would link XMPPError to the stanza, so that it's retrievable from the XMPErrorException. That's what I've done in Flowdalic@45feaec, then adding the callback simply becomes Flowdalic@feff081

Feedback to this solution is very appreciated. If I don't hear anything within the next week, this will go into the official master as it is.

Also note that your title says "Add a Roster callback for error Presences.", while the change is about a callback for error result IQs in response to a roster fetch. Error Presences are already available in the Presences Map.

@damencho
Copy link
Contributor Author

Seems fine. Exception carry all the information that is needed and is up to implementation to extract it :) seems fine. Thanks.

@Flowdalic
Copy link
Member

Fixed with feff081

@Flowdalic Flowdalic closed this Dec 17, 2015
damencho referenced this pull request in jitsi/smack_3_2_2 Dec 18, 2015
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