Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] Not getting GraphQL errors with newest updates. #577

Closed
mkromann opened this issue Jan 15, 2024 · 8 comments
Closed

[Bug] Not getting GraphQL errors with newest updates. #577

mkromann opened this issue Jan 15, 2024 · 8 comments

Comments

@mkromann
Copy link

(I also posted this on the remix template but I think this is probably the right place)

After upgrading to shopify-api v9.0.1 and shopify-app-remix v2.3.0 I am not getting correct graphql errors. Usually I receive an error object with error codes and extensions data. Is the request handled correctly by shopify-app-remix? I am just querying with await admin.graphql(…).

Now this is response when max query cost is exceeding:


 See https://shopify.dev/concepts/about-apis/rate-limits for more information on how the
 cost of a query is calculated.

 To query larger amounts of data with fewer limits, bulk operations should be used instead.
 See https://shopify.dev/api/usage/bulk-operations/queries for usage details.

     at throwFailedRequest (/Users/kromann/dev/barefoot/splay/node_modules/@shopify/shopify-api/lib/clients/common.ts:70:11)
     at NewGraphqlClient.<anonymous> (/Users/kromann/dev/barefoot/splay/node_modules/@shopify/shopify-api/lib/clients/admin/graphql/client.ts:126:25)
     at Generator.next (<anonymous>)
     at fulfilled (/Users/kromann/dev/barefoot/splay/node_modules/tslib/tslib.js:166:62)
     at processTicksAndRejections (node:internal/process/task_queues:95:5) {
   response: Response {
     size: 0,
     [Symbol(Body internals)]: {
       body: <ref *1> ReadableStream {
         _state: 'closed',
         _reader: ReadableStreamDefaultReader {
           _ownerReadableStream: [Circular *1],
           _closedPromise_resolve: undefined,
           _closedPromise_reject: undefined,
           _closedPromise: Promise { undefined },
           _readRequests: SimpleQueue {
             _cursor: 0,
             _size: 0,
             _front: { _elements: [], _next: undefined },
             _back: { _elements: [], _next: undefined }
           }
         },
         _storedError: undefined,
         _disturbed: true,
         _readableStreamController: ReadableStreamDefaultController {
           _controlledReadableStream: [Circular *1],
           _queue: SimpleQueue {
             _cursor: 0,
             _size: 0,
             _front: { _elements: [], _next: undefined },
             _back: { _elements: [], _next: undefined }
           },
           _queueTotalSize: 0,
           _started: true,
           _closeRequested: true,
           _pullAgain: false,
           _pulling: false,
           _strategySizeAlgorithm: undefined,
           _strategyHWM: 1,
           _pullAlgorithm: undefined,
           _cancelAlgorithm: undefined
         }
       },
       type: null,
       size: null,
       boundary: null,
       disturbed: true,
       error: null
     },
     [Symbol(Response internals)]: {
       url: 'https://barefoot-aps.myshopify.com/admin/api/2024-01/graphql.json',
       status: 200,
       statusText: 'OK',
       headers: Headers {},
       counter: 0,
       highWaterMark: 16384
     }
   },
   headers: {
     'Alt-Svc': [ 'h3=":443"; ma=86400' ],
     'Cf-Cache-Status': [ 'DYNAMIC' ],
     'Cf-Ray': [ '8460fdc0e8dcbe42-CPH' ],
     Connection: [ 'close' ],
     'Content-Encoding': [ 'br' ],
     'Content-Language': [ 'en' ],
     'Content-Security-Policy': [
       "default-src 'self' data: blob: 'unsafe-inline' 'unsafe-eval' https://* shopify-pos://*; block-all-mixed-content; child-src 'self' https://* shopify-pos://*; connect-src 'self'
 wss://* https://*; frame-ancestors 'none'; img-src 'self' data: blob: https:; script-src https://cdn.shopify.com https://cdn.shopifycdn.net https://checkout.shopifycs.com https://api.stripe.com
https://mpsnare.iesnare.com https://appcenter.intuit.com https://www.paypal.com https://js.braintreegateway.com https://c.paypal.com https://maps.googleapis.com https://www.google-analytics.com
https://v.shopify.com 'self' 'unsafe-inline' 'unsafe-eval'; upgrade-insecure-requests; report-uri
/csp-report?source%5Baction%5D=query&source%5Bapp%5D=Shopify&source%5Bcontroller%5D=admin%2Fgraphql&source%5Bsection%5D=admin_api&source%5Buuid%5D=3538467d-3481-4d7c-b340-d2ff9e166c37"
     ],
     'Content-Type': [ 'application/json; charset=utf-8' ],
     Date: [ 'Mon, 15 Jan 2024 20:50:28 GMT' ],
     Nel: [
       '{"success_fraction":0.01,"report_to":"cf-nel","max_age":604800}'
     ],
     'Referrer-Policy': [ 'origin-when-cross-origin' ],
     'Report-To': [
       '{"endpoints":[{"url":"https:\\/\\/a.nel.cloudflare.com\\/report\\/v3?s=l%2B02ag0TkliYUiHEXowTquca2H7wK9nYolUK7Z8S0EtZ1lt8OVjjK7wljCC7Ysg2Os6wZGc7pTOYx%2FlUd8ayWcECvAw8%2FUioBa
v%2Fj%2Fp7JfKf%2BuaPIPxj4QNQhXfwlOj7EuWBflT%2FTjs1JN6d"}],"group":"cf-nel","max_age":604800}'
     ],
     Server: [ 'cloudflare' ],
     'Server-Timing': [
       'processing;dur=68, graphql;desc="admin/query/other", cfRequestDuration;dur=174.999952'
     ],
     'Strict-Transport-Security': [ 'max-age=7889238' ],
     'Transfer-Encoding': [ 'chunked' ],
     Vary: [ 'Accept-Encoding' ],
     'X-Content-Type-Options': [ 'nosniff' ],
     'X-Dc': [ 'gcp-europe-north1,gcp-europe-west4' ],
     'X-Download-Options': [ 'noopen' ],
     'X-Envoy-Upstream-Service-Time': [ '71' ],
     'X-Frame-Options': [ 'DENY' ],
     'X-Permitted-Cross-Domain-Policies': [ 'none' ],
     'X-Request-Id': [ '3538467d-3481-4d7c-b340-d2ff9e166c37' ],
     'X-Shardid': [ '340' ],
     'X-Shopid': [ '3130523763' ],
     'X-Shopify-Api-Version': [ '2024-01' ],
     'X-Shopify-Stage': [ 'production' ],
     'X-Sorting-Hat-Podid': [ '340' ],
     'X-Sorting-Hat-Shopid': [ '3130523763' ],
     'X-Stats-Apiclientid': [ '57157156865' ],
     'X-Stats-Apipermissionid': [ '537557205333' ],
     'X-Stats-Userid': [ '71908032627' ],
     'X-Xss-Protection': [
       '1; mode=block;
report=/xss-report?source%5Baction%5D=query&source%5Bapp%5D=Shopify&source%5Bcontroller%5D=admin%2Fgraphql&source%5Bsection%5D=admin_api&source%5Buuid%5D=3538467d-3481-4d7c-b340-d2ff9e166c37'
     ]
   }
 }
@mkromann
Copy link
Author

Why would the request even be throwing? Looking at the docs for v9 here, it seems that a error object should be returned in these cases

This is the function determining to throw: https://github.com/Shopify/shopify-api-js/blob/main/packages/shopify-api/lib/clients/common.ts#L60

@mkromann
Copy link
Author

It might just be me but I have a really hard time navigating your packages.

Anyhow, according to this release note @shopify/shopify-app-remix should have support for the new clients.

And according to this commit in the remix app template, some work have been done to support the type generation. However, I cannot figure out if the loop have been closed to support the new clients. @lizkenyon

This morning I did more testing, and I cannot get any proper errors using the admin query interface.

@paulomarg
Copy link
Contributor

paulomarg commented Jan 16, 2024

Hey, thanks for raising this! Before v9, the GQL client would always throw a GraphqlQueryError if it got back a 200 response that contained an errors field, and that behaviour continued in v9. So the app would always have had to catch that error in order to handle it.

I'm having a little bit of trouble identifying where we would return an error object prior to v9. Could you please share a code sample from before you upgraded your app so I can have a better idea of what we might have broken?

If there is a difference in behaviour, we would definitely want to fix it.

@mkromann
Copy link
Author

Hey, thanks for raising this! Before v9, the GQL client would always throw a GraphqlQueryError if it got back a 200 response that contained an errors field, and that behaviour continued in v9. So the app would always have had to catch that error in order to handle it.

I'm having a little bit of trouble identifying where we would return an error object prior to v9. Could you please share a code sample from before you upgraded your app so I can have a better idea of what we might have broken?

If there is a difference in behaviour, we would definitely want to fix it.

Hi,

I think a PR was just created by some of your colleagues that fixes this in @shopify-api-js here: Shopify/shopify-api-js#1146

If this doesn't work, I'll get back to you with details.

/Magnus

@mkromann
Copy link
Author

@paulomarg I am still not certain how all these packages are interconnected. Can you think of an easy way for me to test the effect of the linked PR?

@paulomarg
Copy link
Contributor

paulomarg commented Jan 17, 2024

Hey! So, what we did recently was to create packages which are standalone clients:

  • admin-api-client works with the Admin API
  • storefront-api-client works with the Storefront API
  • shopify-api is the same as before, but now it uses the standalone clients for consistency, plus support for other things like OAuth, billing, etc.

This way, apps can use only the clients if they're not interested in the other bits (e.g. if your app doesn't need other features from shopify-api, or it's a custom app without background jobs).

I also think you're right - if all you wanted was access to the body (or the errors inside it), that PR will make it possible to use it again in the thrown GraphqlQueryError. That was a mistake on our part, so hopefully this will work for you. Please let us know if it doesn't after we release it :)

@mkromann
Copy link
Author

mkromann commented Feb 1, 2024

The PR fixed my issue — thanks.

@paulomarg
Copy link
Contributor

Amazing, I'm closing this then!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants