fix(json_family): Fix error in JsonFamilyTest.MGet

[BagritsevichStepan]

Until now the test JsonFamilyTest.MGet crashed in the case of jsonpathv2=false due to an incorrect expected error

[romange]

How did it pass the CI checks?

[BagritsevichStepan]

How did it pass the CI checks?

Because it fails only when jsonpathv2=false. I think we can test both cases (when jsonpathv2 false and true)

[kostasrim]

How did it pass the CI checks?

Because it fails only when jsonpathv2=false. I think we can test both cases (when jsonpathv2 false and true)

I don't think it's really worth it to test jsonpathv2=false for this case but I let @romange decide on this :)

Also as a general request, after you create a PR do not force-push such that we can follow the logical changes that are done between commits and comments here. Squash merging your PR will suffice if you do this.

[BagritsevichStepan]

@kostasrim I'd preferred to add logs to the ParsePathV2.

I would like to keep the error (not the logs) in the upper functions, because maybe we want to use their error in other methods

[adiholden]

It is not good to use LOG(WARNING) in commands path for wrong input coming from user, as this can explode the log if user sends many of this commands. We use VLOG(2) for this cases.

[kostasrim]

@BagritsevichStepan maybe it's also worth replacing LOG(WARNING) with VLOG on doc_accessor as well ? Apologies for the back an forth here I should have noticed the hot path and log from the beginning

[BagritsevichStepan]

@kostasrim Maybe it's worth making a separate pr for LOG(warning)? Since this is not really relevant to the current problem

[kostasrim]

no strong preference :)

[BagritsevichStepan]

Fixed in the pr #3293

[romange]

I agree that we should not usually log based on a user input - it's a security risk. However, VLOG(2) is pretty verbose, it may log stuff per request . I think VLOG(1) can work well for such situations, where we may want to set level 1 in production just to see these corner cases without the danger of exploding logs because of other VLOG(2) printings.

[BagritsevichStepan]

Fixed

[adiholden]

please add ./json_test --jsonpathv2=false to the ci.yml in this PR, as Roman recommended in the other PR you opened