ConnectTimeout exception doesn't respect IncludeDetailInExceptions=false

[vonzshik]

I was experimenting with StackExchange.Redis a bit, where at some point I tried to send a query while local redis was down and got an exception with this message:

The message timed out in the backlog attempting to send because no connection became available (5000ms) - Last Connection Exception: It was not possible to connect to the redis server(s) localhost:6379/Interactive. Error connecting right now. To allow this multiplexer to continue retrying until it's able to connect, use abortConnect=false in your connection string or AbortOnConnectFail=false; in your code. ConnectTimeout, command=SET, timeout: 5000, inst: 0, qu: 0, qs: 0, aw: False, bw: CheckingForTimeout, rs: NotStarted, ws: Idle, in: 0, last-in: 0, cur-in: 0, sync-ops: 2, async-ops: 129, serverEndpoint: localhost:6379, conn-sec: n/a, aoc: 1, mc: 1/1/0, mgr: 10 of 10 available, clientName: Some-PC(SE.Redis-v2.9.11.19757), IOCP: (Busy=0,Free=1000,Min=1,Max=1000), WORKER: (Busy=2,Free=32765,Min=24,Max=32767), POOL: (Threads=5,QueuedItems=0,CompletedItems=2864,Timers=8), v: 2.9.11.19757 (Please take a look at this article for some common client-side issues that can cause timeouts: https://stackexchange.github.io/StackExchange.Redis/Timeouts)\r\nIt was not possible to connect to the redis server(s) localhost:6379/Interactive. Error connecting right now. To allow this multiplexer to continue retrying until it's able to connect, use abortConnect=false in your connection string or AbortOnConnectFail=false; in your code. ConnectTimeout

While it's definitely nice and informative, the problem is that I had ConfigurationOptions.IncludeDetailInExceptions set to false, yet I still got pretty much everything, including the server address and a few metrics. Do you think it makes sense to not include common details, like it's done for NoConnectionAvailable exception?

In addition, it seems like ConfigurationOptions.IncludePerformanceCountersInExceptions is completely ignored whatsoever.

[mgravell]

The IncludeDetailInExceptions, as noted, currently aims to avoid leaking user data (key names, values, etc) into exceptions:

Whether exceptions include identifiable details (key names, additional .Data annotations).

Since in many environments PII rules mean that anything resembling user data is actively unwanted in error logs. Infrastructure details like the relevant server is a different category of data - that any infrastructure team will need to investigate a problem. We could suppress the data much further, but this would probably need to be via a different configuration token.

The observation about performance counters is ... more intriguing. I'm investigating.

[mgravell]

On the performance counters question, it looks like we nuked this from orbit 3 years ago, and we no longer access performance-counter data in any code path, at least to the original meaning of IncludePerformanceCountersInExceptions. Would you agree with this statement, @NickCraver ?

That then leaves a few options:

1. mark IncludePerformanceCountersInExceptions as [Obsolete]
2. repurpose it to mean more of the number data (could be confusing for existing setups)