fix: gracefully handle unsettable keys during automocking (#1786)

Sometimes while automocking we will encounter a property that, for some
unknown reason, throws an error when you try to define it. Unfortunately
I can't figure out _why_ it errors, so instead I have opted to quietly
skip these unsettable properties. If that becomes a problem in the
future this can be revisited, but I don't forsee it being an issue since
these keys are mostly deeply internal prototype stuff that 99.999% of
people don't even know exists, let alone want to mock. Plus, if you
_really_ need to have this behavior you can always use `__mocks__` or
just shim it inline yourself.

packages/vitest/src/runtime/mocker.ts

@@ -3,7 +3,7 @@ import { isNodeBuiltin } from 'mlly'
 import { basename, dirname, join, resolve } from 'pathe'
 import { normalizeRequestId, toFilePath } from 'vite-node/utils'
 import type { ModuleCacheMap } from 'vite-node/client'
-import { getAllProperties, getType, getWorkerState, isWindows, mergeSlashes, slash } from '../utils'
+import { getAllMockableProperties, getType, getWorkerState, isWindows, mergeSlashes, slash } from '../utils'
 import { distDir } from '../constants'
 import type { PendingSuiteMock } from '../types/mocker'
 import type { ExecuteOptions } from './execute'
@@ -172,15 +172,24 @@ export class VitestMocker {
     const finalizers = new Array<() => void>()
     const refs = new RefTracker()
 
+    const define = (container: Record<Key, any>, key: Key, value: any) => {
+      try {
+        container[key] = value
+        return true
+      }
+      catch {
+        return false
+      }
+    }
+
     const mockPropertiesOf = (container: Record<Key, any>, newContainer: Record<Key, any>) => {
       const containerType = getType(container)
       const isModule = containerType === 'Module' || !!container.__esModule
-      for (const property of getAllProperties(container)) {
+      for (const { key: property, descriptor } of getAllMockableProperties(container)) {
         // Modules define their exports as getters. We want to process those.
         if (!isModule) {
           // TODO: Mock getters/setters somehow?
-          const descriptor = Object.getOwnPropertyDescriptor(container, property)
-          if (descriptor?.get || descriptor?.set)
+          if (descriptor.get || descriptor.set)
             continue
         }
 
@@ -194,24 +203,27 @@ export class VitestMocker {
         // recursion in circular objects.
         const refId = refs.getId(value)
         if (refId) {
-          finalizers.push(() => newContainer[property] = refs.getMockedValue(refId))
+          finalizers.push(() => define(newContainer, property, refs.getMockedValue(refId)))
           continue
         }
 
         const type = getType(value)
 
         if (Array.isArray(value)) {
-          newContainer[property] = []
+          define(newContainer, property, [])
           continue
         }
 
         const isFunction = type.includes('Function') && typeof value === 'function'
         if ((!isFunction || value.__isMockFunction) && type !== 'Object' && type !== 'Module') {
-          newContainer[property] = value
+          define(newContainer, property, value)
           continue
         }
 
-        newContainer[property] = isFunction ? value : {}
+        // Sometimes this assignment fails for some unknown reason. If it does,
+        // just move along.
+        if (!define(newContainer, property, isFunction ? value : {}))
+          continue
 
         if (isFunction) {
           spyModule.spyOn(newContainer, property).mockImplementation(() => undefined)

packages/vitest/src/utils/base.ts

@@ -5,22 +5,25 @@ function isFinalObj(obj: any) {
   return obj === Object.prototype || obj === Function.prototype || obj === RegExp.prototype
 }
 
-function collectOwnProperties(obj: any, collector: Set<string | symbol>) {
-  const props = Object.getOwnPropertyNames(obj)
-  const symbols = Object.getOwnPropertySymbols(obj)
-
-  props.forEach(prop => collector.add(prop))
-  symbols.forEach(symbol => collector.add(symbol))
+function collectOwnProperties(obj: any, collector: Set<string | symbol> | ((key: string | symbol) => void)) {
+  const collect = typeof collector === 'function' ? collector : (key: string | symbol) => collector.add(key)
+  Object.getOwnPropertyNames(obj).forEach(collect)
+  Object.getOwnPropertySymbols(obj).forEach(collect)
 }
 
-export function getAllProperties(obj: any) {
-  const allProps = new Set<string | symbol>()
+export function getAllMockableProperties(obj: any) {
+  const allProps = new Set<{ key: string | symbol; descriptor: PropertyDescriptor }>()
   let curr = obj
   do {
-    // we don't need propterties from these
+    // we don't need properties from these
     if (isFinalObj(curr))
       break
-    collectOwnProperties(curr, allProps)
+
+    collectOwnProperties(curr, (key) => {
+      const descriptor = Object.getOwnPropertyDescriptor(curr, key)
+      if (descriptor)
+        allProps.add({ key, descriptor })
+    })
     // eslint-disable-next-line no-cond-assign
   } while (curr = Object.getPrototypeOf(curr))
   return Array.from(allProps)

test/core/src/mockedC.ts

@@ -22,3 +22,6 @@ export async function asyncFunc(): Promise<string> {
   await new Promise<void>(resolve => resolve())
   return '1234'
 }
+
+// This is here because mocking streams previously caused some problems (#1671).
+export const exportedStream = process.stderr

test/core/test/mocked.test.ts

@@ -4,7 +4,7 @@ import { value as virtualValue } from 'virtual-module'
 import { two } from '../src/submodule'
 import * as mocked from '../src/mockedA'
 import { mockedB } from '../src/mockedB'
-import { MockedC, asyncFunc } from '../src/mockedC'
+import { MockedC, asyncFunc, exportedStream } from '../src/mockedC'
 import * as globalMock from '../src/global-mock'
 
 vitest.mock('../src/submodule')
@@ -63,3 +63,8 @@ test('async functions should be mocked', () => {
   vi.mocked(asyncFunc).mockResolvedValue('foo')
   expect(asyncFunc()).resolves.toBe('foo')
 })
+
+// This is here because mocking streams previously caused some problems (#1671).
+test('streams', () => {
+  expect(exportedStream).toBeDefined()
+})