Improve expiration handling for client-persisted circuit state

[oroztocil]

When client tries to resume the circuit after the client-held serialized state expires, the operation fails without proper handling. (See comment for details.)

Description

• Moves de-serialization of root component descriptors from UpdateRootComponents earlier to ResumeCircuit to allow checking whether the data is valid and not expired. The operation now explicitly fails if the data is not valid (meaning it could not be used later anyway).
• Fixes bug in calculation of the expiration date for the client-persisted state.
• Adds new E2E test that covers the scenario with expired client-persisted state.
• Updates existing tests to account for the refactoring of ResumeCircuit and UpdateRootComponents.

Fixes #64607

[oroztocil]

Improve expiration handling for client-persisted circuit state

Issue

When client tries to resume the circuit after the client-held serialized state expires, the operation fails without proper handling.

Cause

1. Client calls Blazor.pauseCircuit
2. Server serializes the state and sends it to the client
3. During serialization, we protect the data with expiration date which is set to the maximum of the InMemoryRetentionPeriod and DistributedRetentionPeriod from CircuitOptions. This happens here. Notice that there is a simple bug where we actually don't take the maximum of those two settings, but the InMemoryRetentionPeriod. Fixing this, however, only improves the situation in that the problem would (by default) not occur after 2 hourse, but 8 hours. The actual problem should still be fixed.
4. Client calls Blazor.resumeCircuit, this passes the serialized state containing the expiration mark to the server.
5. Server creates new circuit and attaches the state.
6. ComponentHub.UpdateRootComponents is called on the server which calls CircuitPersistenceManager.ToRootComponentOperationBatch. This returns null instead of the operation batch because ServerComponentDeserializer.TryDeserializeServerComponent fails due to the mark on the serialized data being expired.
7. CircuitHost.UpdateRootComponents throws because the operations are null and there is no check for it.
8. The circuit and client enter a wildly errorneous state: the UI shows no error but interactivity stops working, there are errors in the browser console. Only fix is to reload the page manually.

Solution variants

While serializing the state in PauseCircuit, we can make the client-side state expiration period independent from the server-side state retention period. We can either set it to some reasonably large constant, or make it configurable with a new CircuitOptions property. If not, we should at least fix the bug where the expiration period is not based on the maximum of the Distributed and InMemory retention periods.

Even if we do this, we still should explicitly deal with the possibility of expired client-side state when resuming the circuit. Some locations where we can tackle the issue:

1. When trying to resume the circuit in the client

When client requests pausing the circuit, we can add expiration information along with the serialized state that we return from the server. When trying to resume, client can cooperatively check whether its persisted state is still valid and simply not pass it to the server if it is expired, or inform the user somehow that this is the case. (We don't care about malicious clients, the resume operation will still fail as it does now.)

2. When recieving the client-side state on the server in ResumeCircuit

On the server side, what complicates things is that the complete circuit resume is split into two operations (with separate calls from the client): ResumeCircuit and UpdateRootComponents. ResumeCircuit only attaches the client-provided state in the serialized form to the CircuitHost instance and succeeds even with invalid state - which we could recognize already at this point. Only later, in UpdateRootComponents, we deserialize the state and try to use it which fails for invalid (including expired) state.

We can change ResumeCircuit so that it deserializes the client-provided state and fast fails if the state is invalid. If we do this, we can either:

• a) Add the check to ResumeCircuit but keep the workflow the same, i.e. the state is stored CircuitHost in the serialized form and UpdateRootComponents deserializes it again.
• b) Refactor ResumeCircuit and UpdateRootComponents so that the state is deserialized only in the first.

Furthermore, we need to decide what to do in case of expired client-provided state:

• c) We treat it as a valid scenario, i.e. we do not send a client error and only return a falsy value. This would make it the same as when server-side persistent state is missing (after the recent change). In this case, the client would see this as being rejected by the server and would reload the page automatically.
• d) Send a client error as we do with other non-recoverable resume failures. In this case, the client would show the "Yellow Bar" and stop trying to resume.

await NotifyClientError(Clients.Caller, "...");
Context.Abort();

(Note aside: I think we could benefit from having more robust return values for operations such as ResumeCircuit so that we can communicate different failure modes without the async NotifyClientError call.)

3. When accessing the state in UpdateRootComponents

We can also keep not checking the state in ResumeCircuit (meaning it will succeed with expired state) and simply add an explicit failure case in UpdateRootComponents so that it sends a NotifyClientError and aborts the circuit. This would still be a minor improvement over the current state because the failure would be better communicated to the user.

@javiercn Can I get your thoughts on this? Currently I am leaning towards solution 2b+c. That is, deserialize client-provided state already in ResumeCircuit, check it there, reject the client if expired (=> client reloads), store it deserialized for UpdateRootComponents if valid.

However, it might make sense to simply let the client be aware of the expiration date of its state so that it does not even try to use an expired state.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[maraf]

Looks good to me 👍

[Copilot]

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

[javiercn]

Looks great!