Wrap ForceReconnect around network calls, plus cleanup

[amsoedal]

Purpose

• Give more practical examples of how to use ForceReconnect
• Update all methods to async
• Handle ObjectDisposed exception correctly

Pull Request Type

What kind of change does this Pull Request introduce?

[x] Bugfix
[ ] Feature
[x] Code style update (formatting, local variables)
[x] Refactoring (no functional changes, no api changes)
[ ] Documentation content changes
[ ] Other... Please describe:

Other Information

[philon-msft]

using StackExchange.Redis;

starting in .NET Core 3.1, we no longer need to use Newtonsoft. Can you try to migrate this Core quickstart to use System.Text.Json?

https://docs.microsoft.com/en-us/dotnet/standard/serialization/system-text-json-migrate-from-newtonsoft-how-to?pivots=dotnet-6-0 #Resolved

---

Refers to: quickstart/dotnet-core/Program.cs:3 in 39d03c3. [](commit_id = 39d03c3cc105933d65a293b4504d34bf0d1cf285, deletion_comment = False)

[philon-msft]

    private static IConfigurationRoot Configuration { get; set; }

rather than an accessor, this could be a simple field: private static IConfigurationRoot _configuration; #Resolved

---

Refers to: quickstart/dotnet-core/Program.cs:27 in 39d03c3. [](commit_id = 39d03c3cc105933d65a293b4504d34bf0d1cf285, deletion_comment = False)

[philon-msft]

    public static TimeSpan ReconnectMinInterval => TimeSpan.FromSeconds(60);

// StackExchange.Redis will also be trying to reconnect internally, so limit how often we recreate the ConnectionMultiplexer instance in an attempt to reconnect

---

In reply to: 1007213912

---

Refers to: quickstart/dotnet-core/Program.cs:41 in 39d03c3. [](commit_id = 39d03c3cc105933d65a293b4504d34bf0d1cf285, deletion_comment = False)

[philon-msft]

    public static TimeSpan ReconnectErrorThreshold => TimeSpan.FromSeconds(30);

// If errors occur for longer than this threshold, StackExchange.Redis may be failing to reconnect internally, so we'll recreate the ConnectionMultiplexer instance

---

In reply to: 1007214714

---

Refers to: quickstart/dotnet-core/Program.cs:46 in 39d03c3. [](commit_id = 39d03c3cc105933d65a293b4504d34bf0d1cf285, deletion_comment = False)

[philon-msft]

        {

we need a blank line before this try

---

In reply to: 1007215424

---

Refers to: quickstart/dotnet-core/Program.cs:116 in 39d03c3. [](commit_id = 39d03c3cc105933d65a293b4504d34bf0d1cf285, deletion_comment = False)

[philon-msft]

        TimeSpan elapsedSinceLastReconnect = utcNow - previousReconnectTime;

Let's inline DateTimeOffset.UtcNow in this line like: TimeSpan elapsedSinceLastReconnect = DateTimeOffset.UtcNow- previousReconnectTime;

The we can move the declaration of var utcNow down inside the try block on line 162. That way we're not reusing the variable confusingly

---

In reply to: 1007216599

---

Refers to: quickstart/dotnet-core/Program.cs:141 in 39d03c3. [](commit_id = 39d03c3cc105933d65a293b4504d34bf0d1cf285, deletion_comment = False)

[amsoedal]

Let's inline DateTimeOffset.UtcNow in this line like: TimeSpan elapsedSinceLastReconnect = DateTimeOffset.UtcNow- previousReconnectTime;

@philon-msft Should we change to use DateTime everywhere?

[philon-msft]

I think we want to stick with DateTimeOffset. Unless there's a reason to move to DateTime?

---

In reply to: 1008905243

[amsoedal]

I think we want to stick with DateTimeOffset. Unless there's a reason to move to DateTime?

Since we're using UTC everywhere, I think it would be clearer/less error-prone to stick with DateTime. However in the interest of not making too many changes to the ForceReconnect logic, will leave as-is for now

[philon-msft]

Sounds good. DateTimeOffset actually lets people output/display times in their local time, so might be useful for customers.

---

In reply to: 1010086983

[otto-gebb]

I was going to create an issue, but decided to leave a comment to this PR instead.

The PR introduces code like this:

Interlocked.Exchange(ref _connection, null);
// ...
Interlocked.Exchange(ref _connection, newConnection);

Which is equivalent to

_connection = null;
// ...
_connection = newConnection;

but is less readable. Reference assignments are atomic on all .Net platforms (according to Eric Lippert).

Also, this initial assignment of null to _connection is not needed anymore.

[otto-gebb]

Another issue in this PR is that it wraps ForceReconnect() in a try/catch of ObjectDisposedException, which seems incorrect.

The previous implemetation, where ObjectDisposedException was expected to be thrown by func, seems more logical. The func callback is using the potentially stale instance of _database that may be disposed by ForceReconnect(), ForceReconnect itself does not use a ConnectionMultiplexer that can be disposed.