Log all accesses to toStringTag

[garrettgu10]

One of the questions brought up in #1607 is whether or not the change would be breaking, so we wanted to get some visibility into how many workers actually use the toStringTag property on built-in objects (i.e. ResouceTypes). This would allow us to find out if a worker is getting or setting the property.

Open questions:

• Not sure how to make the accessors simply set and get the actual property on the object, so I currently patch them through to a string property. I don't think this is best practice, and the ugly internal string property does show up when I console.log() the object.
• Is this the right level/type of logging we want for this?

[jasnell]

For this it would be simpler just to store the given value within a jsg::Value member field.

[garrettgu10]

I originally tried doing that, but I couldn't get access to the ResourceTypeBuilder instance from within this function, since I need to be able to give a pointer to it to SetAccessor. Any suggestions?

[garrettgu10]

Also, I think we would have to use the object itself to store the property, since we would want two objets to have separate toStringTags even if they share a single ObjectTemplate.

[kentonv]

What's the motivation for this? (This should be included in the PR and commit descriptions.)

[garrettgu10]

What's the motivation for this? (This should be included in the PR and commit descriptions.)

My bad, I'll add it in.

One of the questions brought up in #1607 is whether or not the change would be breaking, so we wanted to get some visibility into how many workers actually use the toStringTag property on built-in objects (i.e. ResouceTypes). This would allow us to find out if a worker is getting or setting the property.

[kentonv]

Ah, I see.

My opinion: I don't think we should worry about it. This is like worrying that adding a new method to an existing API could beak someone, because they might be checking for the existence of that method. It's extremely unlikely, and if we worry about stuff like that we'll never get anything done. So I'd say it's fine to proceed without this logging change and without a compat flag.

Honestly the more likely breakage is from people expecting that request.toString() returns [object Object] instead of [object Request], but I don't think this change would actually detect that. I still think this isn't likely, though.

[garrettgu10]

I agree, especially since Node and Chromium already set the toStringTags correctly.

FYI, based on my testing, this change would actually detect what you mentioned.

[kentonv]

FYI, based on my testing, this change would actually detect what you mentioned.

I think it will actually log any time anyone calls toString() on a JSG type at all? But people almost certainly do do that (probably by accident, but still), so this log line would be expected to show up immediately, but it doesn't actually tell us if changing the result would break anyone.

[jasnell]

Yep, logging on the get here is not likely to tell us anything useful. Logging on the set would at least tell us if someone is trying to work around the lack of our APIs having a ToStringTag. In my original comment I in the other PR I had indicated that I believed the risk of breaking anyone here was really quite low. Logging like this is how we've made clear determinations in the past about whether a change is likely breaking but I think this is one case where we can likely base the decision off judgement.

[garrettgu10]

Okie dokie, lemme close this and try to merge the other change in then. :-)