enableGraphQLOperationTracking attaches invalid context to network errors

[jmccance]

Platform

iOS

Environment

Production

Installed

Swift Package Manager

Version

8.40.1

Xcode Version

16.1

Did it work on previous versions?

Unknown

Steps to Reproduce

1. Set enableGraphQLOperationTracking = true when configuring the Sentry SDK.
2. Ensure enableNetworkTracking is enabled
3. In a running app, make a GraphQL request that returns a 500

Expected Result

An event should appear in Sentry with the graphql_operation_name in the context. Probably a "GraphQL" context with an "Operation Name" field.

(As a bonus, I'd love it for graphql_operation_name to show up in the tags so that it can be used to differentiate between different request errors. Right now all of our network errors to our API get lumped together into a single Sentry issue.)

Actual Result

The Sentry event includes a warning indicating that the graphql context is invalid because it's a String instead of an object.

From the event:

"_meta": {
  "contexts": {
    "graphql_operation_name": {
      "": {
        "err": [
          [
            "invalid_data",
            {
              "reason": "expected an object"
            }
          ]
        ],
        "val": "Settings"
      }
    }
  }
}

Are you willing to submit a PR?

Sure

[philipphofmann]

@jmccance, our GraphQL support currently is very minimal. We only attach the operation name.

sentry-cocoa/Sources/Swift/Tools/URLSessionTaskHelper.swift

Lines 4 to 20 in d70a7e8

	class URLSessionTaskHelper: NSObject {
	static func getGraphQLOperationName(from task: URLSessionTask?) -> String? {
	guard let task = task else { return nil }
	guard task.originalRequest?.value(forHTTPHeaderField: "Content-Type") == "application/json" else { return nil }
	guard let requestBody = task.originalRequest?.httpBody else { return nil }
	let requestInfo = try? JSONDecoder().decode(GraphQLRequest.self, from: requestBody)
	return requestInfo?.operationName
	}
	}
	private struct GraphQLRequest: Decodable {
	let operationName: String
	}

Can you maybe provide the GraphQL body and the expected outcome you would like to have? Similar to what this test does:

sentry-cocoa/Tests/SentryTests/URLSessionTaskHelperTests.swift

Lines 40 to 56 in d70a7e8

	func testHTTPBodyDataValidGraphQL() {
	let task = makeTask(
	headers: ["Content-Type": "application/json"],
	body: """
	{
	"operationName": "MyOperation",
	"variables": {
	"id": "1234"
	},
	"query": "query MyOperation($id: ID!) { node(id: $id) { id } }"
	}
	"""
	)
	let operationName = URLSessionTaskHelper.getGraphQLOperationName(from: task)
	XCTAssertEqual(operationName, "MyOperation")

[jmccance]

Yep, I understand that it's limited. The bug I'm calling out here is that the current implementation causes a warning in the Sentry dashboard when we use Sentry's built-in HTTP client error tracking feature.

I don't think the issue is in URLSessionTaskHelper from what I can see — the method to get the GraphQL operation name works just fine and correctly enriches the breadcrumbs. I think the problem is in SentryNetworkTracker:

sentry-cocoa/Sources/Sentry/SentryNetworkTracker.m

Lines 458 to 461 in d70a7e8

	if (self.isGraphQLOperationTrackingEnabled) {
	context[@"graphql_operation_name"] =
	[URLSessionTaskHelper getGraphQLOperationNameFrom:sessionTask];
	}

It adds a new value to the event context named graphql_operation_name but sets its value to [URLSessionTaskHelper getGraphQLOperationNameFrom:sessionTask]. From what I can see in the documentation that's not valid because that helper returns a String and contexts have to be an object.

I assume those lines need to be something more like this:

    if (self.isGraphQLOperationTrackingEnabled) {
        NSMutableDictionary<NSString *, id> *graphql = [[NSMutableDictionary alloc] init];
        context[@"graphql"] = graphql;
        graphql[@"operation_name"] = [URLSessionTaskHelper getGraphQLOperationNameFrom:sessionTask];
    }

Forgive my rough knowledge of Objective-C. Happy to take a stab at implementing that change if we think it's correct, though.

[philipphofmann]

Sorry, it was late yesterday for me 🙈. You're totally right @jmccance. Your proposed change makes sense. It would be great if you could open a PR.

[jmccance]

Hey, no worries! I'll take a stab at this and try to get a PR out