Slow down polling on congested traffic

[compulim]

• Do not poll again if the existing poll did not complete yet
• Stop on expired token
  • User need to renew the token and restart a new conversation
• Stop on 404
  • If the bot is deleted, stop the connection and don't retry
• Bump to 0.10.0-0

[billba]

I'm frankly worried about making any changes to this code until there are tests verifying current behavior.

I also don't understand the new expired token behavior. When you say "restart a new conversation" do you mean create a new conversationId? The current behavior allows you to resume an existing conversation, why change that?

[billba]

(Also, if you're going to change behavior in a breaking way, you'll need to change the version number accordingly.)

[compulim]

AFAIK, the resume conversation with renewed token is not working for Web Socket today. Will take that part of code out first.

Agree we need tests. And I am working on the test harness in another branch on my fork. Hope someone can jump in sooner than me to add tests.

(Update: done on reverting token error handling)

[billba]

Why not fix resume conversation with renewed token? Why the change of plans? Has Dan Driscoll reviewed this plan?

(I see that you removed that part of the PR, but I'd still love to understand the plan)

[compulim]

The original plan was to provide appropriate retry/backoff/fatal on 400/404/429, and this is P0. We didn't fix 429 in this one because it was not P0, and it was not well-thought (need input from service team).

Renewed token was discovered while we are fixing 400/404. So it's not actually in the plan. IMO, it's a P1 but S0. I am piggybacking it here, but seems we need more thoughts around it. So I am taking it out for now.

Dan and Vince reviewed this plan on 400/404/429. I think they didn't know about "renewed token" bug yet.

Let's visit the "renew token" in another PR. We also need to revisit the lifetime of connections (and DLJS). Because, IIRC, call to end() doesn't stop everything.

[billba]

Sounds like a solid plan, thanks.

[ckkashyap]

I will do a round of manual testing against local DirectLine and DirectLine in prod to ensure all the changes work as expected.

---

In reply to: 429888497 [](ancestors = 429888497)

[ckkashyap]

I have completed manual validation of all the changes -

1. Ensured that each polling request waits for the the previous one to finish
2. If polling returns 404, then polling stops
3. A 404 returned during refresh token stops further retries unlike previous code.

Encountered only one problem - a 403 returned during polling results in rapid retries. This is a regression.

---

In reply to: 430432369 [](ancestors = 430432369,429888497)