Merge pull request #4726 from Shopify/more-environmental-errors

Further checks for environmental issues

packages/app/src/cli/services/function/binaries.ts

@@ -125,7 +125,7 @@ export async function installBinary(bin: DownloadableBinary) {
         const outputStream = createFileWriteStream(tmpFile)
         await bin.processResponse(responseStream, outputStream)
         await chmod(tmpFile, 0o775)
-        await moveFile(tmpFile, bin.path)
+        await moveFile(tmpFile, bin.path, {overwrite: true})
       })
     },
     async () => {},

packages/cli-kit/src/public/node/error.test.ts

@@ -70,4 +70,12 @@ describe('shouldReportErrorAsUnexpected helper', () => {
   test('returns false for errors that imply environment issues', () => {
     expect(shouldReportErrorAsUnexpected(new Error('EPERM: operation not permitted, scandir'))).toBe(false)
   })
+
+  test('checks errors that have stack dependent environment issues', () => {
+    const error = new Error('Maximum call stack size exceeded')
+    expect(shouldReportErrorAsUnexpected(error)).toBe(true)
+    error.stack = `Error: Maximum call stack size exceeded
+at node_modules/.pnpm/[email protected]/node_modules/stubborn-fs/dist/retryify.js:33:26 attempt`
+    expect(shouldReportErrorAsUnexpected(error)).toBe(false)
+  })
 })

packages/cli-kit/src/public/node/error.ts

@@ -191,7 +191,7 @@ export function shouldReportErrorAsUnexpected(error: unknown): boolean {
     // this means its not one of the CLI wrapped errors
     if (error instanceof Error) {
       const message = error.message
-      return !errorMessageImpliesEnvironmentIssue(message)
+      return !errorMessageImpliesEnvironmentIssue(message, error.stack ?? '')
     }
     return true
   }
@@ -217,20 +217,29 @@ export function cleanSingleStackTracePath(filePath: string): string {
  * There are certain errors that we know are not due to a CLI bug, but are environmental/user error.
  *
  * @param message - The error message to check.
+ * @param stack - The stack trace to check.
  * @returns A boolean indicating if the error message implies an environment issue.
  */
-function errorMessageImpliesEnvironmentIssue(message: string): boolean {
+function errorMessageImpliesEnvironmentIssue(message: string, stack: string): boolean {
   const environmentIssueMessages = [
     'EPERM: operation not permitted, scandir',
     'EACCES: permission denied',
     'EPERM: operation not permitted, symlink',
     'This version of npm supports the following node versions',
-    'EBUSY: resource busy or locked, rmdir',
+    'EBUSY: resource busy or locked',
     'getaddrinfo ENOTFOUND',
     'Client network socket disconnected before secure TLS connection was established',
     'spawn EPERM',
     'socket hang up',
-  ]
-  const anyMatches = environmentIssueMessages.some((issueMessage) => message.includes(issueMessage))
+    'ENOSPC: no space left on device',
+    ['Maximum call stack size exceeded', /stubborn-fs.*retryify/],
+  ] as const
+  const anyMatches = environmentIssueMessages.some((issueMessage) => {
+    if (typeof issueMessage === 'string') {
+      return message.includes(issueMessage)
+    }
+    const [messageCheck, stackCheck] = issueMessage
+    return message.includes(messageCheck) && stackCheck.test(stack)
+  })
   return anyMatches
 }