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

RefreshToken loop should be run even if user user provides streamUrl #50

Closed
harshithkashyap opened this issue Oct 26, 2017 · 2 comments
Closed
Labels
approved Reviewed and ready to start working on it. Will be added to the work queue in the current iteration. P0 Must Fix. Release-blocker triaged Reviewed by the Triage Team

Comments

@harshithkashyap
Copy link

According to the following snippet here, the refreshTokenLoop function is only run if the streamUrl doesn't exist.

if (this.token && this.streamUrl) {
    this.connectionStatus$.next(ConnectionStatus.Online);
     return Observable.of(connectionStatus);
} else {
     return this.startConversation().do(conversation => {
         this.conversationId = conversation.conversationId;
         this.token = this.secret || conversation.token;
         this.streamUrl = conversation.streamUrl;
         this.referenceGrammarId = conversation.referenceGrammarId;
          if (!this.secret)
               this.refreshTokenLoop();
          this.connectionStatus$.next(ConnectionStatus.Online);
}

This is valid when starting a new conversation but when the user is connecting to an existing conversation, a streamUrl is returned while requesting a directline token.

As of now, we'll need to request a directline token for a conversation on the backend, ignore the streamUrl and only send token and conversationId to directline constructor. This would internally call same URL as that of backend to obtain the streamUrl leading to redundant calls.

GET https://directline.botframework.com/v3/directline/conversations/abc123?watermark=0000a-42
@compulim
Copy link
Collaborator

I think what you observe here is correct.

We need to revisit all resume/end/error/expired conversation stories, please track it at #124.

@compulim compulim added P0 Must Fix. Release-blocker triaged Reviewed by the Triage Team 4.3 approved Reviewed and ready to start working on it. Will be added to the work queue in the current iteration. labels Dec 14, 2018
@compulim compulim added 4.4 and removed 4.3 labels Feb 16, 2019
@cwhitten cwhitten removed the 4.4 label Apr 8, 2019
@cwhitten
Copy link
Member

see #124 for updates on this workstream

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Reviewed and ready to start working on it. Will be added to the work queue in the current iteration. P0 Must Fix. Release-blocker triaged Reviewed by the Triage Team
Projects
None yet
Development

No branches or pull requests

4 participants