Infinite reconnect loop can occur due to fatal error that should really close the container

[markfields]

Some examples we've seen in telemetry:

• Attempting to transmit a message larger than socket.io permits results in the connection closing, then we reconnect and try the same op again with the same result.
• The host is providing tokens with the incorrect audience. The server disconnects every time, but the new token we get on reconnect has the same problem.
  • This shouldn't happen, and I don't have record or memory of what was going on here, so I may have been mistaken to include this example
• Server outage results in 500s on JoinSession call, and we never bail out, leaving the user with the Reconnecting banner but not real hope of reconnecting.

One fix would be a mechanism to watch and see if we're hitting the same error N times in a row and close the container.

[markfields]

General symptom in a session is to see DeltaConnectionFailureToConnect event (including the error that occurred) followed by repeated JoinSession_end events (as we try again and again to establish a connection).

Another error we're seeing hit this pattern is very generic, simply a "TransportError" received via the connect_error event on the socket. See #8416 and internally to Msft, incident 275888262.

[andre4i]

Thanks, @markfields.

This is tracked by #8179
#7599
#7545

(See some investigations here: #7599 (comment))

I have a PR open to track the message size but I'd prefer we would have feature gates to enable it..

[markfields]

@andre4i - You're speaking of just the one example it seems?

Any concerns with a world where (until that's fixed) container closes after some time, rather than being stuck in disconnect state?

[jvoels]

For the odsp related consideration "The host is providing tokens with the incorrect audience. The server disconnects every time, but the new token we get on reconnect has the same problem."

The service perhaps should be conveying "non-retriable" for this type of issue. Or the driver, if it runs under the assumption things are retriable by default, may need to deal with this specific error scenario differently.

Outwardly I imagine the UX experiences will usually aspire to a "seamless reconnect" - so the disconnected + open container state supports this.

So I would be concerned about pushing retry logic up the stack when detailed knowledge of what should be considered fatal or not will be closer to the driver.

[markfields]

@jvoels good comment, yes we should take a look at whether these errors are correctly marked as retryable, and whether that's being respected (spoiler alert: In some cases it's not, see #8570)

[vladsud]

I believe this issue is fully addressed by @andre4i and should be closed with his PR.

[andre4i]

#9243

[markfields]

I think it's ok to close, but for the record -- Andrei's fix only addresses the case where the connection can't take hold because of a pending op in the runtime. The case where the host is consistently giving a bad token would not be addressed by that fix.

I'm ok to be data-driven and see what kinds of issues come up in livesite for Loops - or issue reports from others using the ecosystem.

cc @vladsud @andre4i

[vladsud]

I think wrong token will not result in infinite reconnects, as we will get 403/401. These errors are retried once (with new token), and then non-recoverable error that closes container. At least that's how it worked and should work.

Note that t may take one cycle of reconnects to materialize, as we do not treat 403/401 errors as critical when received on disconnect - we always try to reconnect and errors on connection flow are critical

[markfields]

Hmmm I think you're right. I wonder what I saw in telemetry, too bad I didn't link to the logs or provide more details.

[markfields]

The other case recently was around a service outage where JoinSession returned 500 every time. We may want to consider a loader-level mitigation that would force the container to be read-only if we get stuck in this way.

[vladsud]

Yes, 500 might be worth following up. I think we still want to keep reconnecting, but maybe have linger back off period, and maybe eventually (after like couple minutes of retries) to bail out. Maybe other way to say it - I think policy here should be slightly different from what we do with other cases, but I'm not sure we have (today) enough data at container / container runtime layer to differentiate.

[markfields]

The 500's case is tracked in this internal PBI: https://dev.azure.com/office/OC/_workitems/edit/5828431/

[markfields]

Opened #9560 as a way to better address the 500s case.