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

fix(authenticator): await successful retry before setting the session #52

Merged

Conversation

@velrest
Copy link
Contributor

velrest commented Sep 9, 2019

Currently if a retry fails, undefined is set as the session as we do
not return anything in case we start a retry. Now the retries are
blocking and the session is ONLY set if either a retry is successful or
the last retry fails.

@velrest velrest self-assigned this Sep 9, 2019
Currently if a retry fails, `undefined` is set as the session as we do
not return anything in case we start a retry. Now the retries are
blocking and the session is ONLY set if either a retry is successful or
the last retry fails.
@velrest velrest force-pushed the velrest:dont-update-session-on-faulty-retry branch from 867b033 to 18b9c1f Sep 9, 2019
@velrest velrest marked this pull request as ready for review Sep 9, 2019
@velrest

This comment has been minimized.

Copy link
Contributor Author

velrest commented Sep 9, 2019

closes #51

@kaldras
kaldras approved these changes Sep 9, 2019
@velrest velrest merged commit edc42ef into adfinis-sygroup:master Sep 9, 2019
1 check passed
1 check passed
Travis CI - Pull Request Build Passed
Details
return new Promise(resolve => {
later(
this,
() => resolve(this._refresh(refresh_token, retryCount + 1)),

This comment has been minimized.

Copy link
@anehx

anehx Sep 9, 2019

Member

Shouldn't this be awaited?

This comment has been minimized.

Copy link
@velrest

velrest Sep 9, 2019

Author Contributor

the await is in the

this.trigger("sessionDataUpdated", await this._refresh(token));

This comment has been minimized.

Copy link
@anehx

anehx Sep 9, 2019

Member

Yeah but this promise that you return here will resolve immediately, won't it?

@anehx

This comment has been minimized.

Copy link
Member

anehx commented Sep 9, 2019

🎉 This PR is included in version 0.4.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@anehx anehx added the released label Sep 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.