Fix file data streaming for http polling

[msujew]

What it does

Closes #14067

When using socket.io's long polling mechanism, sending messages to the frontend in the backend while responding to a request from the frontend might result in messages arriving in an unexpected order. In particular, it might result in those notifications arriving before the request response arrives.

Usually this is no issue, since we rarely depend on the exact order of messages. However, in the file data streaming, we expect the response before any of the notifications arrive. This seems to be the case with websockets, but not with long polling.

This change makes the frontend in charge of the sequence id handle instead of the backend. This entirely removes the possibility of a race condition without impacting the behavior of the file streaming feature.

How to test

1. Follow the reproduction steps from File streaming broken with socket.io polling #14067.
2. Ensure that files can be successfully opened.

Follow-ups

We might want to look into any potential other places where we depend on notifications + a response for a backend RPC call. I don't think there are any others, but I wouldn't guarantee it.

Review checklist

• As an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• As a reviewer, I agree to behave in accordance with the review guidelines

[tsmaeder]

@msujew I have to disagree that message ordering is not important, see for example: #13960. This is exactly the kind of problem that is totally baffling and seems to appear at randoem. Can we fix the problem in the polling socket implementation so that it respects message ordering?

[msujew]

@tsmaeder I'm not sure this is fixable by "fixing" the polling implementation - polling is by design multiplexed (we always have one polling connection and 0 or more request connections open at a time). Therefore, we would need to fix the backend service to not accidentally send the data before responding from the handler. In fact, it shouldn't require to respond at all.

[msujew]

@tsmaeder I've changed the code so that the race condition observed in the initial post is no longer possible. The frontend is now in charge of the file handle instead of receiving the file handle from the backend.

[tsmaeder]

@msujew maybe I'm misunderstanding the issue, but if I do this in the back end (pseudocode):

frontEnd.sendReply(<someData>);
frontEnd.sendEvent(<some event>);

I expect the front end to receive the reply before the event. Otherwise we'll run into trouble in various places. Is there a reason the polling implementaton of socket.io should work differently than the websocket implemention in this case?

[msujew]

@tsmaeder I've read up on this a bit socket.io guarantees message ordering on both websocket and http fallback. I would generally assume that this works as intended. However, what we've been doing is a bit of a hack anyway: We attempt to respond with a handler id, that we already use in the events, without making any guarantees about the order. This doesn't seem to be a problem in websocket contexts, but likely due to some specific implementation details. I see no reason why this also shouldn't fail on websockets. To explain this in pseudo code:

const response = await service.do(() => {
  frontEnd.sendEvent(someEvent);
});
frontEnd.sendReply(response);

On websockets, sendReply seems to happen first, but on long polling, sendEvent happens first. Our code itself guarantees no order.

The new implementation fixes this issue by specifiying the handler on the frontend level, something that is way cleaner anyway and something that we do throughout the rest of the codebase as well.

[tsmaeder]

Ah, I get it:

1. Front end requests stream read
2. back end starts servicing request
3. back end allocates handle for stream
4. back end starts reading the file, generating strea events (onData) sent to front end
5. back end sends hande as reply to original request.

Just wanted to make sure we don't overlook some deeper problem here.

[tsmaeder]

The only difference I can see for polling is that websocket probably uses some native event mechanism, whereas the polling likely goes through a timeout. That might reorder the execution releative to promises.

[tsmaeder]

@msujew trying to reproduce this with the transport fixed to 'polling', but opening files from the explorer works just fine for me without the fix 🤷

[tsmaeder]

Never mind, I was able to reproduce to for some files in the browser version.