StringBuilder.ToString() throws misleading ArgumentOutOfRangeException for internal chunk inconsistency

[jinek]

Title

StringBuilder.ToString() throws misleading ArgumentOutOfRangeException for internal chunk inconsistency

Description

StringBuilder.ToString() validates internal chunk boundaries before copying:

if ((uint)(chunkLength + chunkOffset) > (uint)result.Length ||
    (uint)chunkLength > (uint)sourceArray.Length)
{
    throw new ArgumentOutOfRangeException(
        nameof(chunkLength),
        SR.ArgumentOutOfRange_IndexMustBeLessOrEqual);
}

This exception is misleading:

• ToString() has no parameters.
• chunkLength is a private local copied from internal StringBuilder state.
• The caller cannot provide or correct a parameter named chunkLength.
• The failed condition indicates an inconsistent internal snapshot, normally caused by concurrent mutation.

A real occurrence was reported in #46022. Concurrent use is caller misuse because StringBuilder is not thread-safe, but reporting it as an invalid argument obscures the real cause.

Expected behavior

Throw an exception representing an invalid or inconsistent operation rather than ArgumentOutOfRangeException with a fictitious parameter name.

This could be an InvalidOperationException, or a suitable internal consistency/concurrency exception translated into an appropriate public exception.

The exact exception type may require compatibility discussion. However, this path should not claim that the caller supplied an invalid chunkLength argument.

Actual behavior

System.ArgumentOutOfRangeException:
Index was out of range. Must be non-negative and less than the size of the collection.
(Parameter 'chunkLength')
   at System.Text.StringBuilder.ToString()

This presents an internal implementation variable as a public method parameter.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

[tannergooding]

Notably there is a parameter, which is this (i.e. sb.ToString() has sb as its only parameter).

This probably could be a bit clearer in terms of message surfaced, but I expect we cannot and won't change the exception type. At best we'd surface that it looks like concurrent usage corruption like some of the collection types do.

[jinek]

Notably there is a parameter, which is this (i.e. sb.ToString() has sb as its only parameter).

We can pass it to nameof then 😂

[jeffhandley]

I agree we would not accept a change to the exception type here. We invite a PR with thoughtful reworking of the exception message.