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

RegistrationManager: Handle failing registration form request #652

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

melvo
Copy link
Contributor

@melvo melvo commented Sep 9, 2024

PR check list:

  • Document your code
  • Add \since QXmpp 1.X, QXMPP_EXPORT
  • Update doc/doap.xml
  • Add unit tests
  • Format the code: Run clang-format -i src/<edited-file(s)> tests/<edited-file(s)>

https://xmpp.org/extensions/xep-0077.html#usecases-cancel specifies the
usage of the stream error <not-authorized/>.
That is now handled as a valid response to the account deletion request
instead of a result IQ stanza or in addition to it.
The same applies to the stream error <conflict> since some servers send
that instead of the <not-authorized/> stream error.

There are servers closing the connection during account creation with a
<connection-timeout/> stream error.
That is now ignored.
This is needed if the server does not respond with a registration form.
That can be the case if too many CAPTCHAs were not solved.
Comment on lines 284 to 299

Q_EMIT registrationFormReceived(iq);
switch (iq.type()) {
case QXmppIq::Result:
info(u"Received registration form."_s);
Q_EMIT registrationFormReceived(iq);
break;
case QXmppIq::Error:
warning(u"Registration form could not be received: "_s.append(iq.error().text()));
Q_EMIT registrationFailed(iq.error());
break;
default:
break; // should never occur
}

return true;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change. Applications that used to listen for IQ errors on registrationFormReceived() won't receive any feedback now.

If you want to split up the error case, create a new signal. But refactoring this to a task-based API would be more useful, I guess.

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

Successfully merging this pull request may close these issues.

2 participants