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

Added errors catch #55

Merged
merged 2 commits into from
Mar 23, 2017
Merged

Added errors catch #55

merged 2 commits into from
Mar 23, 2017

Conversation

EremenkoAndrey
Copy link
Contributor

No description provided.

}
}, options);
} catch (err) {
console.error('FB is not defined');
Copy link
Member

Choose a reason for hiding this comment

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

Why did you assume here that the only error that could happen is FB is not defined?

}
});
} catch (err) {
console.error(err);
Copy link
Member

Choose a reason for hiding this comment

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

Since we already have a promise here, we could reject the promise with the given error instead of logging it to the console.

Please apply this change to the other promises below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it was my extra fix for users with blocked access to Facebook.
You are right, i will change my pull request in the near future

@ihadeed ihadeed merged commit 5d12aba into zyra:master Mar 23, 2017
ihadeed added a commit that referenced this pull request Apr 2, 2017
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