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

don't log error if user has no bookmarks #1822

Merged
merged 1 commit into from
Jan 10, 2020

Conversation

ChaosKid42
Copy link
Contributor

I think converse should not log an error in case the user simply has no bookmarks.

log.error(iq);
}
if (deferred) {
} else if (deferred) {
if (iq.querySelector('error[type="cancel"] item-not-found')) {
// Not an exception, the user simply doesn't have any bookmarks.
window.sessionStorage.setItem(this.fetched_flag, true);
return deferred.resolve();
} else {
return deferred.reject(new Error("Could not fetch bookmarks"));
Copy link
Member

Choose a reason for hiding this comment

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

Here you still need to log an error though, otherwise we won't know what really went wrong.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated accordingly.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, sorry but the changelog entry needs to be moved to the top as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

log.error(iq);
}
if (deferred) {
} else if (deferred) {
if (iq.querySelector('error[type="cancel"] item-not-found')) {
// Not an exception, the user simply doesn't have any bookmarks.
window.sessionStorage.setItem(this.fetched_flag, true);
return deferred.resolve();
} else {
return deferred.reject(new Error("Could not fetch bookmarks"));
Copy link
Member

Choose a reason for hiding this comment

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

Thanks, sorry but the changelog entry needs to be moved to the top as well.

@jcbrand jcbrand merged commit 36e5605 into conversejs:master Jan 10, 2020
@ChaosKid42 ChaosKid42 deleted the fix_error_if_no_bookmarks branch January 10, 2020 22:04
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