Ask to extend ConnectionMultiplexer.IncludeDetailInExceptions check to more places

[ORuban]

ConnectionMultiplexer currently has a IncludeDetailInExceptions flag with the following description

        /// <summary>
        /// Should exceptions include identifiable details? (key names, additional .Data annotations)
        /// </summary>
        public bool IncludeDetailInExceptions { get; set; }

which expected to mean that when IncludeDetailInExceptions=True on multiplexer, it might include command and key, otherwise only command.

Though, it is still possible to receive error messages like below when flag is disabled:

UnableToResolvePhysicalConnection on UNLINK Redis-Key, unable to write batch"

based on code, it may happen due to below logic in ResultProcessor class that doesn't check IncludeDetailInExceptions flag:

public void ConnectionFail(Message message, ConnectionFailureType fail, Exception innerException, string annotation)
{
	PhysicalConnection.IdentifyFailureType(innerException, ref fail);

	string exMessage = fail.ToString() + (message == null ? "" : (" on " + (
		fail == ConnectionFailureType.ProtocolFailure ? message.ToString() : message.CommandAndKey)));
	if (!string.IsNullOrWhiteSpace(annotation)) exMessage += ", " + annotation;

	var ex = innerException == null ? new RedisConnectionException(fail, exMessage)
		: new RedisConnectionException(fail, exMessage, innerException);
	SetException(message, ex);
}

[mgravell]

The main purpose of that flag is to avoid include "data" such as keys in exception messages, which can cause problems in some environments. The message "UnableToResolvePhysicalConnection on UNLINK , unable to write batch" doesn't include any "data" - it just tells you what went wrong in broad terms. I'm not sure there's anything we need to fix there. Can you be more specific as to what problem we want to resolve here?

[ORuban]

Sorry, I wasn't careful enough with text formatting and used <>.
The case is that the actual error message does include key and has form as

UnableToResolvePhysicalConnection on UNLINK Actual_Redis_Key, unable to write batch

cause UNLINK Actual_Redis_Key is what message.CommandAndKey returns

[NickCraver]

@ORuban Ah okay, now I understand the ask. I believe including it in this case is intentional. This isn't a one-off command that failed but something severe enough to kill a connection. In that case, we want to include the key by default to help users debug what may be a massive key fetch and similar problems.

[NickCraver]

@ORuban took another look at this, some time ago we made it on by default so issues should be minimal to none. I put a PR in here if you want to look: #1990 - soon as I can get some eyes on that, we'll get it in for the v2.5 release.