Dd-trace-js: [datadog-plugin-graphql] - Missing loc object breaks query

[rdhaliwal]

Hello! I've come across an interesting scenario that's resulting in some of my GraphQL requests breaking.

Essentially, on L295 of datadog-plugin-graphql/src/index.js, the fieldNode doesn't have loc object, resulting in the error "Cannot read property 'start' of undefined".

If I change L294 to the following, then the request still works:

if (document && fieldNode && fieldNode.loc) {

I'm still in the process of debugging exactly how this happens, the only common part about reproducing it is that this only happens for _some_ schema stitched queries and mutations.

My question/s:

• I'm not overly familiar with the GraphQL AST, but how reasonable is it to assume that a fieldNode _always_ has a loc object?
  
  
  
  • When looking at the graphql-tag library, there is a method that strips this object out sometimes. This to me suggests that it's not something that can be assumed to be there.
  
  
• Does it make sense to update the tracer so that it has to do a check loc exists before accessing it?

Environment

• Operation system: MacOS 10.14.6
• Node version: 8.15.0
• Tracer version: 0.15.0

[rochdev]

That's interesting. I would expect a field node to always have a location to indicate which part of the document it includes. Is there anything else missing from the fieldNode? Depending on the structure of the fieldNode that is causing the issue, we may do different things depending on what it means when fieldNode.loc is missing.

[rdhaliwal]

As far as I can tell, no, everything else is there. It's just the loc that's missing. Here's the object: https://gist.github.com/rdhaliwal/fda91f7db85d5426f5a1cd02de2e5b5e

[rochdev]

What is the value of document for that field?

[rdhaliwal]

@rochdev I've updated the gist with the document. The key difference seems to be the alias is missing in the fieldNode?

If I remove the alias, the query works. However, there are other queries & mutations with aliases that work perfectly fine, and other queries that don't have aliases that also break in a similar manner.

Something else I tried was seeing if there was another field object (maybe the alias created two?). But startResolveSpan is only called by assertField, and when I log the fields array, there's only 1 entry in it.

[rochdev]

From the document, it looks like the field is not actually the entire document. This means that without the loc it's not possible to know what is the source of the field. I'll have to add a check to the fieldNode but this will have the side-effect to lose some information for that field on the span.

I consider this a bug in Apollo since a field that is part of a document should always include a loc, so I'll have to also investigate this further after I release a fix to at least prevent a crash when the bug is present.

[rdhaliwal]

Awesome, thanks for that @rochdev ! Any idea of a rough timeline on when to expect #676 to be merged?

And yes, I agree, I have a feeling there's a bug in Apollo or something else that's messing up the fieldNode. I'm trying to trace it down, but it's slow going >_>

[rochdev]

As soon as it's merged I'll publish a release. It should happen by tomorrow.

[rdhaliwal]

Beautiful, thanks!