Upgrade graphql-request to remove url.parse deprecation warnings

.changeset/graphql-url-parse-warning.md

@@ -0,0 +1,5 @@
+---
+'@shopify/cli-kit': patch
+---
+
+Fix the `url.parse()` deprecation warning during GraphQL requests.

packages/app/package.json

@@ -69,7 +69,7 @@
     "csv-parse": "7.0.2",
     "diff": "5.2.2",
     "esbuild": "0.28.1",
-    "graphql-request": "6.1.0",
+    "graphql-request": "7.4.0",
     "h3": "1.15.11",
     "http-proxy-node16": "1.0.6",
     "ignore": "6.0.2",

packages/app/src/cli/commands/app/execute.test.ts

@@ -74,7 +74,7 @@ test('preserves native GraphQL aliases, order, and UTF-8 on stdout and in output
   expect(response.errors).toBeUndefined()
   const extensions = JSON.parse('{"__proto__":{"trace":"preserved"},"constructor":false,"nullable":null,"empty":[]}')
   vi.mocked(adminRequestDoc).mockImplementation(async ({responseOptions}) => {
-    responseOptions?.onResponse?.({data: response.data, extensions, status: 200, headers: new Headers()})
+    responseOptions?.onResponse?.({data: response.data, extensions, status: 200, headers: new Headers(), body: ''})
     return response.data
   })
   const json = JSON.stringify({data: response.data, extensions}, null, 2)
@@ -126,7 +126,9 @@ test('uses one shared fatal error document and preserves an existing output file
       data: {shop: null},
       extensions: {cost: {actualQueryCost: 1}},
     }
-    vi.mocked(adminRequestDoc).mockRejectedValue(new ClientError({...details, status: 200}, {query}))
+    vi.mocked(adminRequestDoc).mockRejectedValue(
+      new ClientError({...details, status: 200, headers: new Headers(), body: ''}, {query}),
+    )
     vi.stubEnv('SHOPIFY_FLAG_JSON', '1')
     await withCapturedStandardStreams(async ({stdout, stderr}) => {
       await runCommand(['--json', '--output-file', outputFile]).catch(handler)
@@ -141,7 +143,9 @@ test('uses one shared fatal error document and preserves an existing output file
 
 test('preserves the text error banner without a fatal error for a GraphQL failure', async () => {
   const errors = [new GraphQLError('Field invalidField does not exist')]
-  vi.mocked(adminRequestDoc).mockRejectedValue(new ClientError({errors, status: 200}, {query}))
+  vi.mocked(adminRequestDoc).mockRejectedValue(
+    new ClientError({errors, status: 200, headers: new Headers(), body: ''}, {query}),
+  )
   await withCapturedStandardStreams(async ({stdout, stderr}) => {
     await expect(runCommand([])).resolves.toEqual({app})
     expect(stdout()).toBe('')

packages/app/src/cli/commands/app/import/custom-data-definitions.test.ts

@@ -68,7 +68,7 @@ async function withApp(run: (directory: string) => Promise<void>) {
           metafieldDefinitions: {
             pageInfo: {hasNextPage: false, endCursor: null},
             nodes:
-              variables?.ownerType === 'PRODUCT'
+              variables && 'ownerType' in variables && variables.ownerType === 'PRODUCT'
                 ? [
                     {
                       key: 'color',
@@ -127,7 +127,13 @@ test.each([
 test.each([false, true])('distinguishes an empty conversion from denied scope: %s', async (scopeDenied) => {
   await withApp(async (directory) => {
     vi.mocked(adminAsAppRequestDoc).mockImplementation(async ({query, variables}) => {
-      if (scopeDenied && query === MetafieldDefinitions && variables?.ownerType === 'PRODUCT') {
+      if (
+        scopeDenied &&
+        query === MetafieldDefinitions &&
+        variables &&
+        'ownerType' in variables &&
+        variables.ownerType === 'PRODUCT'
+      ) {
         throw new Error('ACCESS_DENIED: Missing access scope')
       }
       return {

packages/app/src/cli/commands/app/release.test.ts

@@ -108,6 +108,7 @@ describe('app release command', () => {
     rawRequest.mockRejectedValueOnce(
       new ClientError(
         {
+          body: '',
           status: 200,
           headers: new Headers(),
           errors: [
@@ -172,7 +173,12 @@ describe('app release command', () => {
     const releaseRequest = vi.spyOn(client, 'release')
     rawRequest.mockRejectedValueOnce(
       new ClientError(
-        {status, headers: new Headers({'x-request-id': 'lookup-request-id'}), errors: [new GraphQLError(message)]},
+        {
+          status,
+          headers: new Headers({'x-request-id': 'lookup-request-id'}),
+          body: '',
+          errors: [new GraphQLError(message)],
+        },
         {query: 'query AppVersionByTag { versionByTag { id } }'},
       ),
     )

packages/app/src/cli/services/execute-operation.test.ts

@@ -203,7 +203,7 @@ describe('executeOperation', () => {
     const data = {store: {name: 'Test Shop'}, value: null, enabled: false}
     const extensions = {cost: {requestedQueryCost: 2}}
     vi.mocked(adminRequestDoc).mockImplementation(async ({responseOptions}) => {
-      responseOptions?.onResponse?.({data, extensions, status: 200, headers: new Headers()})
+      responseOptions?.onResponse?.({data, extensions, status: 200, headers: new Headers(), body: ''})
       return data
     })
 
@@ -220,6 +220,8 @@ describe('executeOperation', () => {
 
   test('retains partial query data and native error extensions', async () => {
     const response = {
+      headers: new Headers(),
+      body: '',
       errors: [new GraphQLError('Access denied', {extensions: {code: 'ACCESS_DENIED'}})],
       data: {shop: null},
       extensions: {cost: {actualQueryCost: 1}},

packages/app/src/cli/services/generate/shop-import/declarative-definitions.test.ts

@@ -1176,7 +1176,12 @@ describe('importDeclarativeDefinitions', () => {
     })
 
     vi.mocked(adminAsAppRequestDoc).mockImplementation(async ({query, variables}) => {
-      if (query === MetafieldDefinitions && variables?.ownerType === 'PRODUCT') {
+      if (
+        query === MetafieldDefinitions &&
+        variables &&
+        'ownerType' in variables &&
+        variables.ownerType === 'PRODUCT'
+      ) {
         return {
           metafieldDefinitions: {
             pageInfo: {hasNextPage: false, endCursor: null},

packages/app/tsconfig.json

@@ -3,6 +3,7 @@
   "include": ["./src/**/*.ts", "./src/**/*.js", "./src/**/*.tsx"],
   "exclude": ["./dist", "./src/templates/**/*"],
   "compilerOptions": {
+    "lib": ["ES2024", "DOM"],
     "outDir": "dist",
     "allowJs": true,
     "rootDir": "src",

packages/cli-kit/package.json

@@ -134,7 +134,7 @@
     "fs-extra": "11.1.0",
     "gradient-string": "2.0.2",
     "graphql": "16.14.2",
-    "graphql-request": "6.1.0",
+    "graphql-request": "7.4.0",
     "h3": "1.15.11",
     "ignore": "6.0.2",
     "ink": "6.8.0",

packages/cli-kit/src/private/node/api.test.ts

@@ -19,6 +19,7 @@ describe('retryAwareRequest', () => {
   test('handles retries', async () => {
     // First give a network error; then a rate limit with an explicit retry; then an unknown rate limit; then a successful call
     const rateLimitedResponseWithRetry = {
+      body: '',
       status: 200,
       errors: [
         {
@@ -33,6 +34,7 @@ describe('retryAwareRequest', () => {
     }
 
     const rateLimitedResponse = {
+      body: '',
       status: 200,
       errors: [
         {
@@ -97,6 +99,7 @@ describe('retryAwareRequest', () => {
     // whose GraphQL error has extensions.code "THROTTLED" — no 429 status, no
     // retry-after header.
     const throttledResponse = {
+      body: '',
       status: 200,
       errors: [
         {
@@ -152,6 +155,7 @@ describe('retryAwareRequest', () => {
     // This test gives a false warning from vitest if fake timers are used. It thinks the exception is uncaught.
     vi.useRealTimers()
     const messageOnlyResponse = {
+      body: '',
       status: 200,
       errors: [
         {
@@ -190,6 +194,7 @@ describe('retryAwareRequest', () => {
     // This test gives a false warning from vitest if fake timers are used. It thinks the exception is uncaught.
     vi.useRealTimers()
     const rateLimitedResponse = {
+      body: '',
       status: 200,
       errors: [
         {
@@ -476,6 +481,7 @@ describe('retryAwareRequest', () => {
 
   test('records retry events when recordRetries is enabled', async () => {
     const rateLimitedResponse = {
+      body: '',
       status: 200,
       errors: [
         {
@@ -531,6 +537,7 @@ describe('retryAwareRequest', () => {
 
   test('does not record retry events when recordRetries is disabled', async () => {
     const rateLimitedResponse = {
+      body: '',
       status: 200,
       errors: [
         {
@@ -585,6 +592,7 @@ describe('retryAwareRequest', () => {
 
   test('records multiple retry events with correct attempt numbers', async () => {
     const rateLimitedResponse = {
+      body: '',
       status: 200,
       errors: [
         {
@@ -644,6 +652,7 @@ describe('retryAwareRequest', () => {
 
   test('records retry events for too many requests status', async () => {
     const rateLimitedResponse = {
+      body: '',
       status: 200,
       errors: [
         {
@@ -699,6 +708,19 @@ describe('retryAwareRequest', () => {
 })
 
 describe('isTransientNetworkError', () => {
+  test.each([
+    Object.assign(new Error('read ECONNRESET'), {code: 'ECONNRESET'}),
+    Object.assign(new Error('other side closed'), {code: 'UND_ERR_SOCKET'}),
+    new AggregateError([Object.assign(new Error('connect failed'), {code: 'ECONNREFUSED'})]),
+  ])('recognizes native fetch network causes: %s', (cause) => {
+    expect(isTransientNetworkError(new TypeError('fetch failed', {cause}))).toBe(true)
+  })
+
+  test('does not retry native fetch configuration errors', () => {
+    const cause = Object.assign(new Error('invalid keep-alive header'), {code: 'UND_ERR_INVALID_ARG'})
+    expect(isTransientNetworkError(new TypeError('fetch failed', {cause}))).toBe(false)
+  })
+
   test('identifies transient network errors that should be retried', () => {
     const transientErrors = [
       'socket hang up',
@@ -770,6 +792,12 @@ describe('isTransientNetworkError', () => {
 })
 
 describe('isNetworkError', () => {
+  test('recognizes native fetch certificate errors without retrying them', () => {
+    const error = new TypeError('fetch failed', {cause: new Error('self-signed certificate')})
+    expect(isNetworkError(error)).toBe(true)
+    expect(isTransientNetworkError(error)).toBe(false)
+  })
+
   test('identifies all transient network errors', () => {
     const transientErrors = ['ECONNRESET', 'ETIMEDOUT', 'ENOTFOUND', 'socket hang up', 'premature close']
 

packages/cli-kit/src/private/node/api.ts

@@ -102,8 +102,9 @@ export function isTransientNetworkError(error: unknown): boolean {
       'timeout',
       'premature close',
       'getaddrinfo',
+      'und_err_socket',
     ]
-    const errorMessage = error.message.toLowerCase()
+    const errorMessage = networkErrorMessage(error)
     const anyMatches = transientErrorMessages.some((issueMessage) => errorMessage.includes(issueMessage))
     const missingReason = /^request to .* failed, reason:\s*$/.test(errorMessage)
     return anyMatches || missingReason
@@ -131,13 +132,25 @@ export function isNetworkError(error: unknown): boolean {
   // Then check for permanent network errors (SSL/TLS/certificate issues)
   if (error instanceof Error) {
     const permanentNetworkErrorMessages = ['certificate', 'cert', 'tls', 'ssl', 'altnames']
-    const errorMessage = error.message.toLowerCase()
+    const errorMessage = networkErrorMessage(error)
     return permanentNetworkErrorMessages.some((issueMessage) => errorMessage.includes(issueMessage))
   }
 
   return false
 }
 
+function networkErrorMessage(error: Error): string {
+  // Native fetch wraps connection and TLS failures in TypeError("fetch failed").
+  const cause =
+    error instanceof TypeError && error.message === 'fetch failed' && error.cause instanceof Error ? error.cause : error
+  const errors = cause instanceof AggregateError ? cause.errors : [cause]
+  return errors
+    .filter((entry): entry is Error => entry instanceof Error)
+    .map((entry) => `${entry.message} ${'code' in entry ? entry.code : ''}`)
+    .join(' ')
+    .toLowerCase()
+}
+
 /**
  * Lower-cased substrings that mean a request was aborted rather than failing on its own.
  * `node-fetch` says 'The user aborted a request.', undici says 'This operation was aborted', and

packages/cli-kit/src/private/node/api/graphql.ts

@@ -52,7 +52,7 @@ function sanitizeDeepVariables(value: unknown, sensitiveKeys: string[]): unknown
     return value.map((item) => sanitizeDeepVariables(item, sensitiveKeys))
   }
 
-  const result: Variables = {}
+  const result: Record<string, unknown> = {}
 
   for (const [key, val] of Object.entries(value)) {
     if (sensitiveKeys.includes(key) && typeof val === 'string') {

packages/cli-kit/src/private/node/api/headers.test.ts

@@ -24,7 +24,6 @@ describe('common API methods', () => {
     const version = CLI_KIT_VERSION
     expect(headers).toEqual({
       'Content-Type': 'application/json',
-      'Keep-Alive': 'timeout=30',
       'X-Shopify-Access-Token': 'Bearer my-token',
       'User-Agent': `Shopify CLI; v=${version}`,
       authorization: 'Bearer my-token',
@@ -45,7 +44,6 @@ describe('common API methods', () => {
       const version = CLI_KIT_VERSION
       expect(headers).toEqual({
         'Content-Type': 'application/json',
-        'Keep-Alive': 'timeout=30',
         'X-Shopify-Access-Token': token,
         'User-Agent': `Shopify CLI; v=${version}`,
         authorization: token,

packages/cli-kit/src/private/node/api/headers.ts

@@ -56,7 +56,6 @@ export function buildHeaders(token?: string): Record<string, string> {
 
   const headers: Record<string, string> = {
     'User-Agent': userAgent,
-    'Keep-Alive': 'timeout=30',
     // 'Sec-CH-UA': secCHUA, This header requires the Git sha.
     'Sec-CH-UA-PLATFORM': process.platform,
     'Content-Type': 'application/json',