fix(parser): correct recursion counting in selection sets

[lrlna]

We've been not quite double maybe more like 1.5 counting nested selection sets in operations with our recursion limit. This PR should provide a fix.

An operation like this should have a recursion high of 4.

query {
    a {
        a {
            a {
                a
            }
        }
    }
}

An operation like this should have a recursion high of 3.

query {
    a {
        a {
            a
        }
    }
}

[lrlna]

no clue why this supposedly had 2 definitions! clearly there is only one. oh well \o/

[lrlna]

I do not remember why we also added to the recursion limit when parsing a field. Do you remember @garypen? Perhaps there was a significant historical reason I am forgetting 🤔

[garypen]

@lrlna My (at this point quite vague) recollection is that we had to enforce recursion limits at various points to prevent stack overflow. i.e.: this mechanism isn't about providing an accurate count of GraphQL component processing; it's actually about preventing excessive stack recursion leading to program termination.

Does that help at all? If not, I'll think harder.

[lrlna]

i think so! it then means incrementing the counter here as well as in the selection set indeed overestimates our recursion counter, which could lead to unexpected stack overflows.

[garypen]

Wouldn't overestimating lead to fewer stack overflows?

[lrlna]

unexpected behaviour*** 🤦‍♂️

[lrlna]

let's say we set a limit of 4_096, and we know that our given operation is exactly 4_096 nested selection sets, we would expect the parser to not abort the operation. but because we are counting in a slightly weird to reason about manner, that operation will abort.

[garypen]

I understand, but that's not the same thing as a stack overflow. The reason we count the way we do is because we are (I think) counting exactly recursive invocations of the parser to prevent stack overflow.

[lrlna]

#519 is the actual fix