[Question] DistributedSession CommitAsync

[csturm83]

I'm a bit confused about how DistributedSession CommitAsync should work. The documentation states that it may throw if the data store is unavailable.

However, I see it exits early here without an exception if the cache is unavailable:

aspnetcore/src/Middleware/Session/src/DistributedSession.cs

Line 270 in a450cb6

if (!IsAvailable)

I suppose an exception could still be thrown in CommitAsync if the cache becomes unavailable after a successful initial Load.

I'll note that the Session Common Errors documentation here seems more strongly worded:

CommitAsync throws an exception if the backing store is unavailable. If CommitAsync fails, the app can process the exception. LoadAsync throws under the same conditions when the data store is unavailable.

I wonder if that may be a bit misleading? My initial interpretation when I read the Common Errors section was that CommitAsync will always fail when the cache is unavailable.

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

[rafikiassumani-msft]

@csturm83 we will be happy to accept a PR that clarifies the doc wording.

[csturm83]

@rafikiassumani-msft I'm not entirely convinced this is a docs issue. I did some more digging into the history of that file and found that the code in question changed in this commit of a PR related to nullability.

@pranavkm Was this change in behavior intentional and/or considered breaking? Should CommitAsync throw always when the cache is unavailable, like it did previously? My honest expectation is that it would, and should.

[cabal95]

I believe the referenced commit also causes CommitAsync() to always deserialize the session, even if the request never accessed it. The getter for "IsAvailable" property calls Load() directly. Which means even if the session hasn't yet been deserialized, and thus no changes made, it will first deserialize it before checking if it has been modified.

[MichalSznajder]

@cabal95 I opened #42009 for issue you mention. I am surprised it was not caught by any regression...

[adityamandaleeka]

#43059 removed the IsAvailable check in CommitAsync and restored the prior behavior here, so I guess this can be closed now.