Fix error processing GeoResult

docs/exp/StringToRedisValue.md

@@ -0,0 +1,18 @@
+StringToRedisValue
+-
+
+Oh dear, it looks like you fell into a `DEBUG` warning. **Don't panic!** You're probably fine.
+
+Short version, *if you're in the core library when seeing this*:
+
+1. If you're *writing* data (to the server), you're probably fine - just add `.AsRedisValue()` to say "trust me bro"; all strings are valid as `RedisValue` content - you're fine. If the value
+   is a literal (i.e. `"some const string"`) or a `const`, you might want to look at `RedisLiterals` and the `.FromRaw(...)` API, which can be more efficient, but otherwise: you do you!
+2. However, if you're *reading* data (from the server), then you need to be careful - there's a good chance you've used `reader.ReadString()` when you meant to use `reader.ReadRedisValue()`.
+
+The impact of *2* would be that *string* payloads work fine, but you silently corrupt *binary* payloads. Additionally, if the value is *small* or an obvious integer, `RedisValue` may
+be more efficient (zero-allocation), and even for non-trivial text-like payloads: using `RedisValue` defers the UTF8 decode cost to the caller. So: ask yourself - is this actually
+a `string` in all cases?
+
+This warning only exists in local `DEBUG` builds - it doesn't exist in the public package.
+
+If you're actually in an auxiliary package (tests, benchmarks, etc): we care less. Feel free to add `<NoWarn>$(NoWarn);StringToRedisValue</NoWarn>` to the csproj. 

src/RESPite.Benchmark/RESPite.Benchmark.csproj

@@ -16,6 +16,7 @@
     <Copyright>2025 - $([System.DateTime]::Now.Year) Marc Gravell</Copyright>
     <!-- for for example: dotnet build /p:TargetVer=3 -->
     <TargetVer>3</TargetVer>
+    <NoWarn>$(NoWarn);StringToRedisValue</NoWarn>
   </PropertyGroup>
 
   <ItemGroup>

src/RESPite/Shared/Experiments.cs

@@ -5,15 +5,21 @@
     // where SomeFeature has the next label, for example "SER042", and /docs/exp/SER042.md exists
     internal static class Experiments
     {
-        public const string UrlFormat = "https://stackexchange.github.io/StackExchange.Redis/exp/";
+        // note: {0} is substituted with the DiagnosticId by the analyzer, e.g. .../exp/SER002
+        public const string UrlFormat = "https://stackexchange.github.io/StackExchange.Redis/exp/{0}";
 
         // ReSharper disable InconsistentNaming
         public const string Server_8_4 = "SER002";
         public const string Server_8_6 = "SER003";
         public const string Respite = "SER004";
         public const string UnitTesting = "SER005";
         public const string Server_8_8 = "SER006";
+
         // ReSharper restore InconsistentNaming
+
+        // this one is not a real experiment; it exists to help me
+        // spot bad API uses, via a DEBUG symbol
+        public const string StringToRedisValue = "StringToRedisValue";
     }
 }
 

src/StackExchange.Redis/APITypes/ClientKillFilter.cs

@@ -152,17 +152,17 @@ internal List<RedisValue> ToList(bool withReplicaCommands)
         if (Username != null)
         {
             parts.Add(RedisLiterals.USERNAME);
-            parts.Add(Username);
+            parts.Add(Username.AsRedisValue());
         }
         if (Endpoint != null)
         {
             parts.Add(RedisLiterals.ADDR);
-            parts.Add((RedisValue)Format.ToString(Endpoint));
+            parts.Add(Format.ToString(Endpoint).AsRedisValue());
         }
         if (ServerEndpoint != null)
         {
             parts.Add(RedisLiterals.LADDR);
-            parts.Add((RedisValue)Format.ToString(ServerEndpoint));
+            parts.Add(Format.ToString(ServerEndpoint).AsRedisValue());
         }
         if (SkipMe != null)
         {

src/StackExchange.Redis/APITypes/GeoPosition.cs

@@ -7,12 +7,12 @@ namespace StackExchange.Redis;
 /// </summary>
 public readonly struct GeoPosition : IEquatable<GeoPosition>
 {
-    internal static string GetRedisUnit(GeoUnit unit) => unit switch
+    internal static RedisValue GetRedisUnit(GeoUnit unit) => unit switch
     {
-        GeoUnit.Meters => "m",
-        GeoUnit.Kilometers => "km",
-        GeoUnit.Miles => "mi",
-        GeoUnit.Feet => "ft",
+        GeoUnit.Meters => RedisLiterals.m,
+        GeoUnit.Kilometers => RedisLiterals.km,
+        GeoUnit.Miles => RedisLiterals.mi,
+        GeoUnit.Feet => RedisLiterals.ft,
         _ => throw new ArgumentOutOfRangeException(nameof(unit)),
     };
 

src/StackExchange.Redis/ArrayGrepRequest.cs

@@ -367,7 +367,7 @@ private static void AddIndex(in MessageWriter writer, RedisArrayIndex? index, Re
         protected override void WriteImpl(in MessageWriter writer)
         {
             writer.WriteHeader(Command, ArgCount);
-            writer.WriteBulkString(key);
+            writer.Write(key);
             if (request.IsReversed)
             {
                 AddIndex(writer, request.End, "$1\r\n+\r\n"u8);

src/StackExchange.Redis/Configuration/LoggingTunnel.cs

@@ -193,7 +193,7 @@ public static async Task<long> ReplayAsync(Stream @out, Stream @in, Action<Redis
                 // so we see the message that had a corrupted reply
                 if (sent.Result is not null)
                 {
-                    pair(sent.Result, RedisResult.Create(ex.Message, ResultType.Error));
+                    pair(sent.Result, RedisResult.Create(ex.Message.AsRedisValue(), ResultType.Error));
                 }
                 throw; // still surface the original exception
             }

src/StackExchange.Redis/ConnectionMultiplexer.Sentinel.cs

@@ -247,7 +247,7 @@ public ConnectionMultiplexer GetSentinelMasterConnection(ConfigurationOptions co
                 try
                 {
                     isPrimary = connection.CommandMap.IsAvailable(RedisCommand.ROLE)
-                        ? server.Role()?.Value == RedisLiterals.master
+                        ? server.Role()?.Value == Role.LabelForMaster
                         : !server.IsReplica;
                 }
                 catch

src/StackExchange.Redis/ConnectionMultiplexer.cs

@@ -231,7 +231,7 @@ internal async Task MakePrimaryAsync(ServerEndPoint server, ReplicationChangeOpt
             }
 
             var nodes = _serverSnapshot; // same as GetServerSnapshot(), but doesn't force span
-            RedisValue newPrimary = Format.ToString(server.EndPoint);
+            var newPrimary = Format.ToString(server.EndPoint);
 
             // try and write this everywhere; don't worry if some folks reject our advances
             if (RawConfig.TryGetTieBreaker(out var tieBreakerKey)
@@ -242,7 +242,7 @@ internal async Task MakePrimaryAsync(ServerEndPoint server, ReplicationChangeOpt
                 {
                     if (!node.IsConnected || node.IsReplica) continue;
                     log?.LogInformationAttemptingToSetTieBreaker(new(node.EndPoint));
-                    msg = Message.Create(0, flags | CommandFlags.FireAndForget, RedisCommand.SET, tieBreakerKey, newPrimary);
+                    msg = Message.Create(0, flags | CommandFlags.FireAndForget, RedisCommand.SET, tieBreakerKey, newPrimary.AsRedisValue());
                     try
                     {
                         await node.WriteDirectAsync(msg, ResultProcessor.DemandOK).ForAwait();
@@ -267,7 +267,7 @@ internal async Task MakePrimaryAsync(ServerEndPoint server, ReplicationChangeOpt
             if (!tieBreakerKey.IsNull && !server.IsReplica)
             {
                 log?.LogInformationResendingTieBreaker(new(server.EndPoint));
-                msg = Message.Create(0, flags | CommandFlags.FireAndForget, RedisCommand.SET, tieBreakerKey, newPrimary);
+                msg = Message.Create(0, flags | CommandFlags.FireAndForget, RedisCommand.SET, tieBreakerKey, newPrimary.AsRedisValue());
                 try
                 {
                     await server.WriteDirectAsync(msg, ResultProcessor.DemandOK).ForAwait();
@@ -298,7 +298,7 @@ async Task BroadcastAsync(ServerSnapshot serverNodes)
                     {
                         if (!node.IsConnected) continue;
                         log?.LogInformationBroadcastingViaNode(new(node.EndPoint));
-                        msg = Message.Create(-1, flags | CommandFlags.FireAndForget, RedisCommand.PUBLISH, channel, newPrimary);
+                        msg = Message.Create(-1, flags | CommandFlags.FireAndForget, RedisCommand.PUBLISH, channel, newPrimary.AsRedisValue());
                         await node.WriteDirectAsync(msg, ResultProcessor.Int64).ForAwait();
                     }
                 }

src/StackExchange.Redis/ExtensionMethods.cs

@@ -172,9 +172,15 @@ public static class ExtensionMethods
             }
 
             if (values.Length == 0) return Array.Empty<RedisValue>();
-            return Array.ConvertAll(values, x => (RedisValue)x);
+            return Array.ConvertAll(values, x => x.AsRedisValue());
         }
 
+        // just like the implicit conversion operator, but by making it
+        // explicit, we're making it very clear that this is intentional,
+        // and avoiding the DEBUG check
+        internal static RedisValue AsRedisValue(this string? value)
+            => value is null ? RedisValue.Null : new(value);
+
         /// <summary>
         /// Create an array of strings from an array of values.
         /// </summary>

src/StackExchange.Redis/Increx.IncrexMessage.cs

@@ -32,7 +32,7 @@ public override int ArgCount
         protected override void WriteImpl(in MessageWriter writer)
         {
             writer.WriteHeader(Command, ArgCount);
-            writer.WriteBulkString(Key);
+            writer.Write(Key);
             WriteIncrementKindAndValue(writer);
             WriteBounds(writer);
             WriteOptions(writer);

src/StackExchange.Redis/KeyspaceIsolation/KeyPrefixed.cs

@@ -938,10 +938,10 @@ protected RedisValue ToInner(RedisValue outer) =>
             RedisKey.ConcatenateBytes(Prefix, null, (byte[]?)outer);
 
         protected RedisValue SortByToInner(RedisValue outer) =>
-            (outer == "nosort") ? outer : ToInner(outer);
+            (outer == RedisLiterals.nosort) ? outer : ToInner(outer);
 
         protected RedisValue SortGetToInner(RedisValue outer) =>
-            (outer == "#") ? outer : ToInner(outer);
+            (outer == RedisLiterals.HashSymbol) ? outer : ToInner(outer);
 
         [return: NotNullIfNotNull("outer")]
         protected RedisValue[]? SortGetToInner(RedisValue[]? outer)

src/StackExchange.Redis/MessageWriter.cs

@@ -80,12 +80,29 @@ internal void Write(in RedisChannel channel)
     internal void WriteBulkString(in RedisValue value)
         => WriteBulkString(value, _writer);
 
+    [MethodImpl(MethodImplOptions.AggressiveInlining)]
+    internal void WriteBulkString(string? value)
+    {
+        if (value is null)
+        {
+            WriteRaw(NullBulkString);
+        }
+        else if (value.Length is 0)
+        {
+            WriteRaw(EmptyBulkString);
+        }
+        else
+        {
+            WriteUnifiedPrefixedString(_writer, null, value);
+        }
+    }
+
     internal static void WriteBulkString(in RedisValue value, IBufferWriter<byte> writer)
     {
         switch (value.Type)
         {
             case RedisValue.StorageType.Null:
-                WriteUnifiedBlob(writer, (byte[]?)null);
+                writer.Write(NullBulkString);
                 break;
             case RedisValue.StorageType.Int64:
                 WriteUnifiedInt64(writer, value.OverlappedValueInt64);

src/StackExchange.Redis/PublicAPI/Debug/PublicAPI.txt

@@ -0,0 +1,2 @@
+#nullable enable
+[StringToRedisValue]static StackExchange.Redis.RedisValue.implicit operator StackExchange.Redis.RedisValue(string? value) -> StackExchange.Redis.RedisValue

src/StackExchange.Redis/PublicAPI/PublicAPI.Shipped.txt

@@ -1837,7 +1837,6 @@ static StackExchange.Redis.RedisValue.implicit operator StackExchange.Redis.Redi
 static StackExchange.Redis.RedisValue.implicit operator StackExchange.Redis.RedisValue(int? value) -> StackExchange.Redis.RedisValue
 static StackExchange.Redis.RedisValue.implicit operator StackExchange.Redis.RedisValue(long value) -> StackExchange.Redis.RedisValue
 static StackExchange.Redis.RedisValue.implicit operator StackExchange.Redis.RedisValue(long? value) -> StackExchange.Redis.RedisValue
-static StackExchange.Redis.RedisValue.implicit operator StackExchange.Redis.RedisValue(string? value) -> StackExchange.Redis.RedisValue
 static StackExchange.Redis.RedisValue.implicit operator StackExchange.Redis.RedisValue(System.Memory<byte> value) -> StackExchange.Redis.RedisValue
 static StackExchange.Redis.RedisValue.implicit operator StackExchange.Redis.RedisValue(System.ReadOnlyMemory<byte> value) -> StackExchange.Redis.RedisValue
 static StackExchange.Redis.RedisValue.implicit operator StackExchange.Redis.RedisValue(uint value) -> StackExchange.Redis.RedisValue

src/StackExchange.Redis/PublicAPI/Release/PublicAPI.txt

@@ -0,0 +1,2 @@
+#nullable enable
+static StackExchange.Redis.RedisValue.implicit operator StackExchange.Redis.RedisValue(string? value) -> StackExchange.Redis.RedisValue

src/StackExchange.Redis/RedisDatabase.VectorSets.cs

@@ -86,7 +86,7 @@ public bool VectorSetRemove(RedisKey key, RedisValue member, CommandFlags flags
 
     public bool VectorSetSetAttributesJson(RedisKey key, RedisValue member, string attributesJson, CommandFlags flags = CommandFlags.None)
     {
-        var msg = Message.Create(Database, flags, RedisCommand.VSETATTR, key, member, attributesJson);
+        var msg = Message.Create(Database, flags, RedisCommand.VSETATTR, key, member, attributesJson.AsRedisValue());
         return ExecuteSync(msg, ResultProcessor.Boolean);
     }
 
@@ -178,7 +178,7 @@ public Task<bool> VectorSetRemoveAsync(RedisKey key, RedisValue member, CommandF
 
     public Task<bool> VectorSetSetAttributesJsonAsync(RedisKey key, RedisValue member, string attributesJson, CommandFlags flags = CommandFlags.None)
     {
-        var msg = Message.Create(Database, flags, RedisCommand.VSETATTR, key, member, attributesJson);
+        var msg = Message.Create(Database, flags, RedisCommand.VSETATTR, key, member, attributesJson.AsRedisValue());
         return ExecuteAsync(msg, ResultProcessor.Boolean);
     }
 
@@ -205,7 +205,7 @@ static RedisValue GetTerminator(RedisValue value, Exclude exclude, bool isStart)
             if (value.IsNull) return isStart ? RedisLiterals.MinusSymbol : RedisLiterals.PlusSymbol;
             var mask = isStart ? Exclude.Start : Exclude.Stop;
             var isExclusive = (exclude & mask) != 0;
-            return (isExclusive ? "(" : "[") + value;
+            return ((isExclusive ? "(" : "[") + value).AsRedisValue();
         }
 
         var from = GetTerminator(start, exclude, true);

src/StackExchange.Redis/RedisDatabase.cs

@@ -1311,15 +1311,16 @@ private sealed class KeyMigrateCommandMessage : Message.CommandKeyBase // MIGRAT
             private readonly MigrateOptions migrateOptions;
             private readonly int timeoutMilliseconds;
             private readonly int toDatabase;
-            private readonly RedisValue toHost, toPort;
+            private readonly string toHost;
+            private readonly int toPort;
 
             public KeyMigrateCommandMessage(int db, RedisKey key, EndPoint toServer, int toDatabase, int timeoutMilliseconds, MigrateOptions migrateOptions, CommandFlags flags)
                 : base(db, flags, RedisCommand.MIGRATE, key)
             {
                 if (toServer == null) throw new ArgumentNullException(nameof(toServer));
                 if (!Format.TryGetHostPort(toServer, out string? toHost, out int? toPort)) throw new ArgumentException($"Couldn't get host and port from {toServer}", nameof(toServer));
                 this.toHost = toHost;
-                this.toPort = toPort;
+                this.toPort = toPort.Value;
                 if (toDatabase < 0) throw new ArgumentOutOfRangeException(nameof(toDatabase));
                 this.toDatabase = toDatabase;
                 this.timeoutMilliseconds = timeoutMilliseconds;
@@ -4392,7 +4393,7 @@ private static RedisValue GetRange(double value, Exclude exclude, bool isStart)
             {
                 if ((exclude & Exclude.Stop) == 0) return value; // inclusive is default
             }
-            return "(" + Format.ToString(value); // '(' prefix means exclusive
+            return ("(" + Format.ToString(value)).AsRedisValue(); // '(' prefix means exclusive
         }
 
         private Message GetRestoreMessage(RedisKey key, byte[] value, TimeSpan? expiry, CommandFlags flags)
@@ -4898,7 +4899,7 @@ private Message GetStreamPendingMessagesMessage(RedisKey key, RedisValue groupNa
             values[offset++] = groupName;
             if (minIdleTimeInMs is not null)
             {
-                values[offset++] = "IDLE";
+                values[offset++] = RedisLiterals.IDLE;
                 values[offset++] = minIdleTimeInMs;
             }
             values[offset++] = minId ?? StreamConstants.ReadMinValue;
@@ -5480,7 +5481,7 @@ protected override void WriteImpl(in MessageWriter writer)
             {
                 writer.WriteHeader(Command, 2);
                 writer.WriteRaw("$4\r\nLOAD\r\n"u8);
-                writer.WriteBulkString((RedisValue)Script);
+                writer.WriteBulkString(Script);
             }
             public override int ArgCount => 2;
         }
@@ -5675,12 +5676,12 @@ protected override void WriteImpl(in MessageWriter writer)
                 else if (asciiHash != null)
                 {
                     writer.WriteHeader(RedisCommand.EVALSHA, 2 + keys.Length + values.Length);
-                    writer.WriteBulkString((RedisValue)asciiHash);
+                    writer.WriteBulkString(asciiHash);
                 }
                 else
                 {
                     writer.WriteHeader(RedisCommand.EVAL, 2 + keys.Length + values.Length);
-                    writer.WriteBulkString((RedisValue)script);
+                    writer.WriteBulkString(script);
                 }
                 writer.WriteBulkString(keys.Length);
                 for (int i = 0; i < keys.Length; i++)
@@ -5757,15 +5758,15 @@ private static Message CreateSortedSetRangeStoreMessage(
 
             RedisValue formattedStart = exclude switch
             {
-                Exclude.Both or Exclude.Start => $"({start}",
-                _ when sortedSetOrder == SortedSetOrder.ByLex => $"[{start}",
+                Exclude.Both or Exclude.Start => $"({start}".AsRedisValue(),
+                _ when sortedSetOrder == SortedSetOrder.ByLex => $"[{start}".AsRedisValue(),
                 _ => start,
             };
 
             RedisValue formattedStop = exclude switch
             {
-                Exclude.Both or Exclude.Stop => $"({stop}",
-                _ when sortedSetOrder == SortedSetOrder.ByLex => $"[{stop}",
+                Exclude.Both or Exclude.Stop => $"({stop}".AsRedisValue(),
+                _ when sortedSetOrder == SortedSetOrder.ByLex => $"[{stop}".AsRedisValue(),
                 _ => stop,
             };
 

src/StackExchange.Redis/RedisKey.cs

@@ -222,7 +222,7 @@ public override int GetHashCode()
 
         internal RedisValue AsRedisValue()
         {
-            if (KeyPrefix == null && KeyValue is string keyString) return keyString;
+            if (KeyPrefix == null && KeyValue is string keyString) return keyString.AsRedisValue();
             return (byte[]?)this;
         }
 

src/StackExchange.Redis/RedisLiterals.cs

@@ -48,9 +48,11 @@ public static readonly RedisValue
             GETKEYS = RedisValue.FromRaw("GETKEYS"u8),
             GETNAME = RedisValue.FromRaw("GETNAME"u8),
             GT = RedisValue.FromRaw("GT"u8),
+            HashSymbol = RedisValue.FromRaw("#"u8),
             HISTORY = RedisValue.FromRaw("HISTORY"u8),
             ID = RedisValue.FromRaw("ID"u8),
             IDX = RedisValue.FromRaw("IDX"u8),
+            IDLE = RedisValue.FromRaw("IDLE"u8),
             IDLETIME = RedisValue.FromRaw("IDLETIME"u8),
             IDMP = RedisValue.FromRaw("IDMP"u8),
             IDMPAUTO = RedisValue.FromRaw("IDMPAUTO"u8),
@@ -75,8 +77,10 @@ public static readonly RedisValue
             MIN = RedisValue.FromRaw("MIN"u8),
             MINMATCHLEN = RedisValue.FromRaw("MINMATCHLEN"u8),
             MODULE = RedisValue.FromRaw("MODULE"u8),
+            NO = RedisValue.FromRaw("NO"u8),
             NODES = RedisValue.FromRaw("NODES"u8),
             NOSAVE = RedisValue.FromRaw("NOSAVE"u8),
+            nosort = RedisValue.FromRaw("nosort"u8),
             NOT = RedisValue.FromRaw("NOT"u8),
             NOVALUES = RedisValue.FromRaw("NOVALUES"u8),
             NUMPAT = RedisValue.FromRaw("NUMPAT"u8),
@@ -165,7 +169,6 @@ public static readonly RedisValue
 
             // misc (config, etc)
             databases = RedisValue.FromRaw("databases"u8),
-            master = RedisValue.FromRaw("master"u8),
             no = RedisValue.FromRaw("no"u8),
             normal = RedisValue.FromRaw("normal"u8),
             pubsub = RedisValue.FromRaw("pubsub"u8),