Rest Api from Cubejs gets mistaken as GraphQL when using both in one handler

[brantleyenglish]

Prerequisites

• I confirm my issue is not in the opened issues
• I confirm the Frequently Asked Questions didn't contain the answer to my issue

Environment check

• I'm using the latest msw version
• I'm using Node.js version 14 or higher

Node.js version

14.17.5

Reproduction repository

https://github.com/brantleyenglish/msw-test

Reproduction steps

Run yarn then yarn test

Current behavior

Currently I am getting this error, and the mocked rest endpoint does not get populated with mock data.

console.error
[MSW] Failed to intercept a GraphQL request to "GET https://azure-bonobo.aws-us-west-2.cubecloudapp.dev/cubejs-api/v1/load": cannot parse query. 
See the error message from the parser below.
Syntax Error: Expected Name, found String "measures".

This url should be parsed as a rest endpoint. This is not a graphql query.

Expected behavior

I expect the test to pass the resultSet to return some form of data.

[kettanaito]

Hey, @brantleyenglish. Thanks for reporting this!

I've posted an update on the way to tell apart GraphQL requests reliably here: #535 (comment)

This really moves the discussion (and the correct solution to this issue) to the GraphQL community. I'd stay in the loop to see how I can help. I'd love to merge a fix for this in a form of relying on the application/graphql+json content type header.

[brantleyenglish]

Wow, this is very cool 😮 !! Thank you for taking a look into my issue and suggesting the improvement.

[kettanaito]

Looks like the URQL community is for this change. We've teamed up to update existing GraphQL server implementations to make sure they can praise application/graphql+json request bodies properly. I'm excited about this.

[kettanaito]

Sadly, the GraphQL community is tentative in adopting the GraphQL over HTTP spec as of now. I'm conflicted here: I don't wish to add constant workarounds which will never work reliably because there's no reliable way to distinguish a GraphQL request.

[kettanaito]

The only approach I see right now, until all the clients respect application/graphql content-type header, is to remove error propagation during potential GraphQL request parsing from MSW:

msw/src/utils/internal/parseGraphQLRequest.ts

Lines 161 to 172 in 66c3ad8

	if (parsedResult instanceof Error) {
	const requestPublicUrl = getPublicUrlFromRequest(request)
	throw new Error(
	devUtils.formatMessage(
	'Failed to intercept a GraphQL request to "%s %s": cannot parse query. See the error message from the parser below.\n\n%s',
	request.method,
	requestPublicUrl,
	parsedResult.message,
	),
	)
	}

Pros:

• Ambiguous REST requests will not throw an error.
• The underlying request matching logic will continue as if the request is not GraphQL if there are any exceptions during its parsing (exceptions are not forwarded to the user).

Cons:

• Decreases confidence from the mocks you write because MSW will contribute less to spotting invalid GraphQL requests (i.e. those with an unterminated query string).

Currently, there's nothing I can recommend to circumvent the runtime exception apart from adjusting your request query/body not to resemble that of GraphQL. That is an intrusive suggestion and that's why I'm not expecting you to apply it.

I wanted to suggest the following thing initially:

The other suggestion is, whenever you have an ambiguous request like this one, make sure that there's a REST request handler that's defined before any other GraphQL handler, if you have any:

export const handlers = [
  rest.post('/cubejs-api/v1/load', (req, res, ctx) => {
    req.url.searchParams.get('query')
    // Make sure to handle this request!
    return res(ctx.json({ hello: 'world' }))
  }),
  graphql.query('AnyQuery', resolver)
]

GraphQL request parsing happens as a part of parse of each handler. Handlers are executed in order (left to right), so if there's a matching REST handler, any other GraphQL handlers won't even begin parsing that request—it's already been resolved.

But then I've learned that request parsing happens before handlers are run (during predicate):

msw/src/handlers/GraphQLHandler.ts

Line 160 in 66c3ad8

predicate(request: MockedRequest, parsedResult: ParsedGraphQLRequest) {

Because request matching may require information otherwise unavailable in the request without an additional parsing step, such as operation type/name of a GraphQL request or path parameters of a REST request. So the suggestion above is irrelevant.

[shunxing]

Hello there :)

I was wondering whether you had any workaround for REST calls with a query in the body not getting caught/recognized by the graphQL MSW handlers when using REST and graphql handlers in the same server

[mattcosta7]

Hello there :)

I was wondering whether you had any workaround for REST calls with a query in the body not getting caught/recognized by the graphQL MSW handlers when using REST and graphql handlers in the same server

I believe that if you define the rest handlers before the graphql handlers it should be doable.

the bigger annoyance is using server.use you may need to add rest handlers again, before the new graphql handler)

[mattcosta7]

@brantleyenglish et al

In version 2.0.9 we shipped #1871 which should help differentiate graphql from rest requests and avoid the console.error discussed here.

That version allows us to bail out of request parsing early if the endpoint is not specifically designated as a graphql endpoing when using graphql.link() and then utilizing the handlers returned from that binding instead of the raw utilities.

To modify existing code:

import {graphql, http} from 'msw'

+ const exampleGraphqlApi = graphql.link("https://example.com/graphql")

const worker = setupWorker(
-  graphql.query('GetUser', ...),
+  exampleGraphqlApi.query('GetUser', ...),
  http.post("https://example.com/users", ({ request }) => {
     const {query} = await request.json()
  })
)

Before, we'd parse requests to https://example.com/users when using the raw graphql.query to see if it was compatible. After 2.0.9 with this change we'll avoid parsing if the endpoint is not defined via that link property.

Using that version or later, and graphql.link should make this more easy to understand and allow you to fix this issue