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/question: E2E testing with GraphQL gives a 401 error #1756

Open
Ulset opened this issue Nov 12, 2024 · 5 comments · May be fixed by #1772
Open

Bug/question: E2E testing with GraphQL gives a 401 error #1756

Ulset opened this issue Nov 12, 2024 · 5 comments · May be fixed by #1772

Comments

@Ulset
Copy link

Ulset commented Nov 12, 2024

Hi we are working on a shopify app, and as our codebase grows the need for testing E2E rises.

Im currently trying to implement some very basic testing of our backend queries to Shopify. Im initalizing the shopify wrapper using the shopifyApp function and everything runs smoothly. After initalizing the wrapper with test config:

const prismaSessionStorage = new PrismaSessionStorage(prisma);

let config: Parameters<typeof shopifyApp>[0] | null

if (process.env.NODE_ENV === 'test') {
  config = testConfig({
    sessionStorage: prismaSessionStorage
  });
} else {
   // Actual config
}
const shopify_handler = shopifyApp(config);

The following test runs smoothly:

test("Verify that queries are okay", async () => {
  const someSession = await prisma.session.findFirstOrThrow()
  const {admin} = await shopify_handler.unauthenticated.admin(someSession.shop)
  const shop = await admin.rest.get({path: 'shop.json'}).then((r) => r.json())
  console.log(shop)
})

I can see in my logs my current dev store
Image

The problem seems to arise when i try to fetch anything from the graphql API

Given the following modified code

test("Verify that queries are okay", async () => {
  const someSession = await prisma.session.findFirstOrThrow()
  const {admin} = await shopify_handler.unauthenticated.admin(someSession.shop)
  const shop = await admin.graphql(`
    #graphql
    query {
      shop {
        currencyCode
        myshopifyDomain
        shopOwnerName
        email
        contactEmail
        plan {
          partnerDevelopment
        }
      }
    }
  `).then((r) => r.json()).then((data) => {
    return data.data.shop
  }).catch((e) => {
    console.error('Error fetching shop info', e)
    return null
  })
  console.log(shop)
})

I seem to get a 401 unauthenticated error:
Image

Since the rest api calls work as expected i would presume the setup is okay, am i doing something wrong?

@sle-c
Copy link
Contributor

sle-c commented Nov 14, 2024

Hi @Ulset ,

I tried to replicate the test with the following code

import "dotenv/config";
import { test } from "@jest/globals";
import prisma from "../app/db.server";
import { unauthenticated } from "../app/shopify.server";

test("Verify that queries are okay", async () => {
  const someSession = await prisma.session.findFirstOrThrow();
  const { admin } = await unauthenticated.admin(someSession.shop);
  const shop = await admin.rest.get({ path: "shop.json" }).then((r) => r.json());
  console.log(shop);
});

test("Verify that gql queries are okay", async () => {
  const someSession = await prisma.session.findFirstOrThrow();
  const { admin } = await unauthenticated.admin(someSession.shop);
  const shop = await admin.graphql(`
    #graphql
    query {
      shop {
        currencyCode
        myshopifyDomain
        shopOwnerName
        email
        contactEmail
        plan {
          partnerDevelopment
        }
      }
    }
  `).then((r) => r.json()).then((data) => {
    return data.data.shop
  }).catch((e) => {
    console.error('Error fetching shop info', e)
    return null
  })
  console.log(shop)
})

When I ran the test, both test cases passed. I used the default shopifyApp config from the template's shopify.server file. I didn't have a separate test configuration. Can you check if the sessions are still valid for the test config you used?

Image

Si

@Ulset
Copy link
Author

Ulset commented Nov 18, 2024

The sessions should be valid, im using vitest and not jest though.

By using the regular config i get this error:
Process finished with exit code 139 (interrupted by signal 11:SIGSEGV)
The config looks like this:

const shopify_handler = shopifyApp({
  apiKey: process.env.SHOPIFY_API_KEY,
  apiSecretKey: process.env.SHOPIFY_API_SECRET || "",
  apiVersion: '2024-10' as ApiVersion,
  scopes: process.env.SCOPES?.split(","),
  appUrl: process.env.SHOPIFY_APP_URL || "",
  authPathPrefix: "/auth",
  sessionStorage: prismaSessionStorage,
  distribution: AppDistribution.AppStore,
  isEmbeddedApp: true,
  billing: {
    [BILLING_PLANS.STARTER]: {
      lineItems: [{
        amount: 24,
        currencyCode: 'USD',
        interval: BillingInterval.Every30Days,
      }]
    },
    [BILLING_PLANS.GROWTH]: {
      lineItems: [{
        amount: 44,
        currencyCode: 'USD',
        interval: BillingInterval.Every30Days
      }]
    },
    [BILLING_PLANS.SCALE]: {
      lineItems: [{
        amount: 84,
        currencyCode: 'USD',
        interval: BillingInterval.Every30Days
      }]
    },
    [BILLING_PLANS.PLUS]: {
      lineItems: [{
        amount: 199,
        currencyCode: 'USD',
        interval: BillingInterval.Every30Days
      }]
    }
  },
  restResources,
  webhooks: webhooks,
  hooks: {
    afterAuth: async ({session, admin}) => {
      const logger = getLogger({session});
      shopify_handler.registerWebhooks({session}).catch(logger.error);
      addWeebeDiscountFunction({admin, session}).catch(logger.error);
    }
  },
  future: {
    unstable_newEmbeddedAuthStrategy: true,
  },
  logger: {level: process.env.NODE_ENV === 'development' ? LogSeverity.Info : LogSeverity.Error},
});

By swapping my custom config to the testConfig provided by the package like this:

const shopify_handler = shopifyApp(testConfig({sessionStorage: prismaSessionStorage}));

Im back to 401
Image

Also, the test actually passes because of the .catch handling, can you verify that the logs outputted by the test is a valid Shop object? Or you can try removing the .catch clause, this test fails:

test("Verify that gql queries are okay", async () => {
  const someSession = await prisma.session.findFirstOrThrow();
  const { admin } = await unauthenticated.admin(someSession.shop);
  const shop = await admin.graphql(`
    #graphql
    query {
      shop {
        currencyCode
        myshopifyDomain
        shopOwnerName
        email
        contactEmail
        plan {
          partnerDevelopment
        }
      }
    }
  `).then((r) => r.json()).then((data) => {
    return data.data.shop
  })
  console.log(shop)
})

@sle-c
Copy link
Contributor

sle-c commented Nov 18, 2024

Hi @Ulset ,

@lizkenyon and I looked into this issue deeper and we found an inconsistent behaviour between the REST client and GraphQL Client. Thank you for reporting this issue and the testConfig detail!

TLDR of the issue:

  • In the REST client, we have logic to use the real session token from the passed in session. Although it also check for another access token in the config before using the session access token.
  • In the GraphQL client, we do not have that logic and it always use the other access token in the config which leads to a 401.

Next steps:

  • We'll put in a fix to make the 2 clients behave the same way for consistency.
  • Ideally, when using testConfig, one should not be able to make a request to a real production shop. It's intended for unit testing. The requests should be mocked.
    • For E2E tests, one should use a real config to a production shop, not a test config.

@Ulset
Copy link
Author

Ulset commented Nov 24, 2024

Great to hear!
Would it be possible to rethink the logic while keeping the core functionality intact? The Shopify CLI sets many environment variables during local development. If the test requires a valid configuration to pass, it seems I’d need to manually log in to the Partner Dashboard and set those variables for my tests to work, right?

Could the Shopify CLI provide an interface for running tests using a specific shopify.toml file? We currently have 3–4 separate .toml files for different staging environments, so this would simplify the process.

@sle-c
Copy link
Contributor

sle-c commented Dec 20, 2024

If the test requires a valid configuration to pass, it seems I’d need to manually log in to the Partner Dashboard and set those variables for my tests to work, right?

In this case you're running an E2E test, so I assume we want to mimic the production environment as close as possible. Therefore, you'll need a valid configuration for that.

Could the Shopify CLI provide an interface for running tests using a specific shopify.toml file? We currently have 3–4 separate .toml files for different staging environments, so this would simplify the process.

This could be a feature request for the CLI package, I'd recommend checking with https://github.com/Shopify/cli

Would it be possible to rethink the logic while keeping the core functionality intact?

We found that this PR can potentially impact a large number of apps #1772 negatively so we're holding off for now

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