Home Original page

Initialize ScriptEvalMessage with the correct command by martinpotter · Pull Request #1930 · StackExchange/StackExchange.Redis

@martinpotter

Currently the constructor for ScriptEvalMessage that takes a SHA1 hash, and sends the EVALSHA command, initializes the base Message class's command to RedisCommand.EVAL instead of RedisCommand.EVALSHA.

@mgravell

wow; looks like we missed a critical test here - presumably this never worked! good eyes

@mgravell

could you please also add a line to releasenotes.md (you'll see the pattern); if you're feeling especially generous, it would also be great to add a test here to show it working, but: if not, I can go back and add some here

@martinpotter

wow; looks like we missed a critical test here

As far as I can tell, this really is only an issue for things like profiling (OpenTelemetry, NR, etc) that log the Command since ScriptEvalMessage overrides WriteImpl and [correctly writes] (

if (hexHash != null)
{
physical.WriteHeader(RedisCommand.EVALSHA, 2 + keys.Length + values.Length);
physical.WriteSha1AsHex(hexHash);
}
) RedisCommand.EVALSHA to the connection.

@mgravell

@martinpotter that probably makes it even more useful to have a test, honestly :)

@martinpotter

@martinpotter

@NickCraver

Missed the force push here, thanks for the test update! Kicking off builds :)

NickCraver

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, thanks for the catch and the fix!

NickCraver added a commit that referenced this pull request

Dec 31, 2021

@NickCraver