rebing / graphql-laravel

Laravel wrapper for Facebook's GraphQL
MIT License
2.13k stars 266 forks source link

APQ #780

Closed edgarsn closed 3 years ago

edgarsn commented 3 years ago

Versions:

Description:

I was trying to implement APQ in my site. Everythink works well until I change:

const httpLinkWithPersistedQuery = createPersistedQueryLink().concat(createHttpLink({
    uri: '/graphql',
}));

to

const httpLinkWithPersistedQuery = createPersistedQueryLink({useGETForHashedQueries: true}).concat(createHttpLink({
    uri: '/graphql',
}));

According to Apollo documentation, it should work.

In this case response is GraphQL error: Syntax Error: Unexpected <EOF>. Full response:

{
  "errors": [
    {
      "message": "Syntax Error: Unexpected <EOF>",
      "extensions": {
        "category": "graphql"
      },
      "locations": [
        {
          "line": 1,
          "column": 1
        }
      ]
    }
  ]
}

I can confirm, that I don't use TrimStrings middleware and run php artisan cache:clear before setting {useGETForHashedQueries: true}

Steps To Reproduce:

I think I described the steps enough in description :)

Let me know if I somehow can help you to debug this issue.

mfn commented 3 years ago

At first I thought it's a json parse error, but actually I think it's a graphql parse error.

Are you able to debug the exact HTTP request being sent? They might help pinpoint the problem. Or debug server side where this is thrown exactly would also help (enabling debug should give a stacktrace)

mfn commented 3 years ago

useGETForHashedQueries

This might well be related to our request parsing .. in 7.x this was done hand-crafted in the library, for the upcoming 8.x release I switched to using https://github.com/laragraph/utils which I noticed does a few things differently. So maybe it better supports this.

You can try https://github.com/rebing/graphql-laravel/releases/tag/8.0.0-rc2 but it requires some effort as it contains quite some breaking changes.

Nevertheless, knowing how the request looks exactly would help writing a testcase to cover this. I personally don't do frontend stuff, so I can't help with this library.

@illambo is the original author, maybe he has some idea here

illambo commented 3 years ago

Hello, I would need at least the trace of the request to better understand the situation and replicate a possible test on the matter.

edgarsn commented 3 years ago

Hello!

{useGETForHashedQueries: false}:

Screenshot from Telescope ![useGetForHashedQueries_false](https://user-images.githubusercontent.com/6625918/118943087-f9db6680-b95b-11eb-9679-0820ccf83063.png)

Payload:

{
    "operationName": null,
    "variables": [],
    "extensions": {
        "persistedQuery": {
            "version": 1,
            "sha256Hash": "d7b7fc27911575ba2b55aa3063148d7979668fb35a19a7e3c2e29c0044c8301e"
        }
    }
}

{useGETForHashedQueries: true}:

Screenshot from Telescope ![useGETForHashedQueries_true](https://user-images.githubusercontent.com/6625918/118943217-17a8cb80-b95c-11eb-938e-2a42617909c0.png)

Request Path: /graphql?extensions=%7B%22persistedQuery%22%3A%7B%22version%22%3A1%2C%22sha256Hash%22%3A%2244e3d137628ea65df1c0103431c93d55782571d9e1a53cf9b597fb007d4b6f86%22%7D%7D&amp;variables=%7B%7D

Stacktrace from laravel logs ``` [2021-05-20 11:08:00] local.ERROR: Syntax Error: Unexpected {"exception":"[object] (GraphQL\\Error\\SyntaxError(code: 0): Syntax Error: Unexpected at /home/vagrant/code/vendor/webonyx/graphql-php/src/Language/Parser.php:413) [stacktrace] #0 /home/vagrant/code/vendor/webonyx/graphql-php/src/Language/Parser.php(527): GraphQL\\Language\\Parser->unexpected() #1 /home/vagrant/code/vendor/webonyx/graphql-php/src/Language/Parser.php(484): GraphQL\\Language\\Parser->parseDefinition() #2 /home/vagrant/code/vendor/webonyx/graphql-php/src/Language/Parser.php(448): GraphQL\\Language\\Parser->GraphQL\\Language\\{closure}() #3 /home/vagrant/code/vendor/webonyx/graphql-php/src/Language/Parser.php(486): GraphQL\\Language\\Parser->many() #4 /home/vagrant/code/vendor/webonyx/graphql-php/src/Language/Parser.php(192): GraphQL\\Language\\Parser->parseDocument() #5 /home/vagrant/code/vendor/webonyx/graphql-php/src/GraphQL.php(135): GraphQL\\Language\\Parser::parse() #6 /home/vagrant/code/vendor/webonyx/graphql-php/src/GraphQL.php(94): GraphQL\\GraphQL::promiseToExecute() #7 /home/vagrant/code/vendor/rebing/graphql-laravel/src/GraphQL.php(132): GraphQL\\GraphQL::executeQuery() #8 /home/vagrant/code/vendor/rebing/graphql-laravel/src/GraphQL.php(115): Rebing\\GraphQL\\GraphQL->queryAndReturnResult() #9 /home/vagrant/code/vendor/rebing/graphql-laravel/src/GraphQLController.php(95): Rebing\\GraphQL\\GraphQL->query() #10 /home/vagrant/code/vendor/rebing/graphql-laravel/src/GraphQLController.php(63): Rebing\\GraphQL\\GraphQLController->executeQuery() #11 /home/vagrant/code/vendor/laravel/framework/src/Illuminate/Routing/Controller.php(54): Rebing\\GraphQL\\GraphQLController->query() #12 /home/vagrant/code/vendor/laravel/framework/src/Illuminate/Routing/ControllerDispatcher.php(45): Illuminate\\Routing\\Controller->callAction() #13 /home/vagrant/code/vendor/laravel/framework/src/Illuminate/Routing/Route.php(254): Illuminate\\Routing\\ControllerDispatcher->dispatch() #14 /home/vagrant/code/vendor/laravel/framework/src/Illuminate/Routing/Route.php(197): Illuminate\\Routing\\Route->runController() #15 /home/vagrant/code/vendor/laravel/framework/src/Illuminate/Routing/Router.php(695): Illuminate\\Routing\\Route->run() #16 /home/vagrant/code/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(128): Illuminate\\Routing\\Router->Illuminate\\Routing\\{closure}() #17 /home/vagrant/code/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(103): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}() #18 /home/vagrant/code/vendor/laravel/framework/src/Illuminate/Routing/Router.php(697): Illuminate\\Pipeline\\Pipeline->then() #19 /home/vagrant/code/vendor/laravel/framework/src/Illuminate/Routing/Router.php(672): Illuminate\\Routing\\Router->runRouteWithinStack() #20 /home/vagrant/code/vendor/laravel/framework/src/Illuminate/Routing/Router.php(636): Illuminate\\Routing\\Router->runRoute() #21 /home/vagrant/code/vendor/laravel/framework/src/Illuminate/Routing/Router.php(625): Illuminate\\Routing\\Router->dispatchToRoute() #22 /home/vagrant/code/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(166): Illuminate\\Routing\\Router->dispatch() #23 /home/vagrant/code/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(128): Illuminate\\Foundation\\Http\\Kernel->Illuminate\\Foundation\\Http\\{closure}() #24 /home/vagrant/code/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/TransformsRequest.php(21): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}() #25 /home/vagrant/code/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/ConvertEmptyStringsToNull.php(31): Illuminate\\Foundation\\Http\\Middleware\\TransformsRequest->handle() #26 /home/vagrant/code/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(167): Illuminate\\Foundation\\Http\\Middleware\\ConvertEmptyStringsToNull->handle() #27 /home/vagrant/code/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/ValidatePostSize.php(27): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}() #28 /home/vagrant/code/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(167): Illuminate\\Foundation\\Http\\Middleware\\ValidatePostSize->handle() #29 /home/vagrant/code/vendor/laravel/framework/src/Illuminate/Foundation/Http/Middleware/PreventRequestsDuringMaintenance.php(86): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}() #30 /home/vagrant/code/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(167): Illuminate\\Foundation\\Http\\Middleware\\PreventRequestsDuringMaintenance->handle() #31 /home/vagrant/code/vendor/fruitcake/laravel-cors/src/HandleCors.php(52): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}() #32 /home/vagrant/code/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(167): Fruitcake\\Cors\\HandleCors->handle() #33 /home/vagrant/code/vendor/fideloper/proxy/src/TrustProxies.php(57): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}() #34 /home/vagrant/code/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(167): Fideloper\\Proxy\\TrustProxies->handle() #35 /home/vagrant/code/orion/core/src/Foundation/Http/Middleware/HandlePreflight.php(33): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}() #36 /home/vagrant/code/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(167): Orion\\Core\\Foundation\\Http\\Middleware\\HandlePreflight->handle() #37 /home/vagrant/code/vendor/laravel/framework/src/Illuminate/Pipeline/Pipeline.php(103): Illuminate\\Pipeline\\Pipeline->Illuminate\\Pipeline\\{closure}() #38 /home/vagrant/code/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(141): Illuminate\\Pipeline\\Pipeline->then() #39 /home/vagrant/code/vendor/laravel/framework/src/Illuminate/Foundation/Http/Kernel.php(110): Illuminate\\Foundation\\Http\\Kernel->sendRequestThroughRouter() #40 /home/vagrant/code/public/index.php(52): Illuminate\\Foundation\\Http\\Kernel->handle() #41 {main} "} ```

Let me know if you need something more.

mfn commented 3 years ago

I suspected correctly in https://github.com/rebing/graphql-laravel/issues/780#issuecomment-844714415 : the handling of the GET request in master/8.x-dev branch already works correctly because we switched to https://github.com/laragraph/utils for handling the specifics of HTTP requests:

This can't be backported to 7.x as it brings breaking changes (see https://github.com/rebing/graphql-laravel/pull/739 for details) and is one of the very reasons we're bumping the major version with the next release.

I'm already running https://github.com/rebing/graphql-laravel/releases/tag/8.0.0-rc2 in production so it's "safe" from that POV, besides the adaptions it requires due to breaking changes.

However: the work for 8.0 is not yet done and more breaking changes might come. If you're not interested in handling further breaking changes, I suggest to wait.

Thanks for your report!


TL;DR: closing, as it already works correctly in master

illambo commented 3 years ago

Sorry if I was not helpful in time. Great job!