fix(import): wrong error message when a module exists but fails

The importer is responsible of importing and patching modules with
configurable fallbacks. If one is missing, it goes to the next one.
Previously if a module existed but was failing when loading it with
`require`, it'd report as missing instead of failing. Now it bubbles the
error correctly.

Author: huafu

src/config/config-set.ts

@@ -354,7 +354,7 @@ export class ConfigSet {
     let hooksFile = process.env.TS_JEST_HOOKS
     if (hooksFile) {
       hooksFile = resolve(this.cwd, hooksFile)
-      return importer.tryThese(hooksFile) || {}
+      return importer.tryTheseOr(hooksFile, {})
     }
     return {}
   }

src/types.ts

@@ -126,13 +126,6 @@ export interface CreateJestPresetOptions {
  */
 export type ModulePatcher<T = any> = (module: T) => T
 
-/**
- * @internal
- */
-export interface TsJestImporter {
-  tryThese(moduleName: string, ...fallbacks: string[]): any
-}
-
 /**
  * Common TypeScript interfaces between versions.
  */

src/util/importer.spec.ts

@@ -3,22 +3,21 @@ import * as fakers from '../__helpers__/fakers'
 
 import { Importer, __requireModule } from './importer'
 
-const requireModule = jest.fn(
-  mod =>
-    mod in modules
-      ? modules[mod]()
-      : (() => {
-          const err: any = new Error(`Module not found: ${mod}.`)
-          err.code = 'MODULE_NOT_FOUND'
-          throw err
-        })(),
-)
-__requireModule(requireModule as any)
+const moduleNotFound = (mod: string) => {
+  const err: any = new Error(`Module not found: ${mod}.`)
+  err.code = 'MODULE_NOT_FOUND'
+  throw err
+}
+const fakeFullPath = (mod: string) => `/root/${mod}.js`
+const requireModule = jest.fn(mod => (mod in modules ? modules[mod]() : moduleNotFound(mod)))
+const resolveModule = jest.fn(mod => (mod in modules ? fakeFullPath(mod) : moduleNotFound(mod)))
+__requireModule(requireModule as any, resolveModule)
 
 let modules!: { [key: string]: () => any }
 beforeEach(() => {
   modules = {}
   requireModule.mockClear()
+  resolveModule.mockClear()
 })
 
 describe('instance', () => {
@@ -30,12 +29,60 @@ describe('instance', () => {
   })
 })
 
-describe('tryTheese', () => {
-  it('tries until it find one not failing', () => {
+describe('tryThese', () => {
+  it('should try until it finds one existing', () => {
     modules = {
-      success: () => 'success',
+      success: () => 'ok',
     }
-    expect(new Importer().tryThese('fail1', 'fail2', 'success')).toBe('success')
+    expect(new Importer().tryThese('missing1', 'missing2', 'success')).toMatchInlineSnapshot(`
+Object {
+  "exists": true,
+  "exports": "ok",
+  "given": "success",
+  "path": "/root/success.js",
+}
+`)
+  })
+  it('should return the error when one is failing', () => {
+    modules = {
+      fail1: () => {
+        throw new Error('foo')
+      },
+      success: () => 'ok',
+    }
+    const res = new Importer().tryThese('missing1', 'fail1', 'success')
+    expect(res).toMatchObject({
+      exists: true,
+      error: expect.any(Error),
+      given: 'fail1',
+      path: fakeFullPath('fail1'),
+    })
+    expect(res).not.toHaveProperty('exports')
+    expect((res as any).error.message).toMatch(/\bfoo\b/)
+  })
+})
+
+describe('tryTheseOr', () => {
+  it('should try until it find one not failing', () => {
+    expect(new Importer().tryTheseOr(['fail1', 'fail2', 'success'])).toBeUndefined()
+    expect(new Importer().tryTheseOr(['fail1', 'fail2', 'success'], 'foo')).toBe('foo')
+    modules = {
+      success: () => 'ok',
+    }
+    expect(new Importer().tryTheseOr(['fail1', 'fail2', 'success'])).toBe('ok')
+    modules.fail2 = () => {
+      throw new Error('foo')
+    }
+    expect(new Importer().tryTheseOr(['fail1', 'fail2', 'success'], 'bar', true)).toBe('bar')
+  })
+  it('should fail if one is throwing', () => {
+    modules = {
+      success: () => 'ok',
+      fail2: () => {
+        throw new Error('foo')
+      },
+    }
+    expect(() => new Importer().tryTheseOr(['fail1', 'fail2', 'success'], 'bar')).toThrow(/\bfoo\b/)
   })
 })
 
@@ -49,10 +96,10 @@ describe('patcher', () => {
       foo: () => ({ foo: true }),
       bar: () => ({ bar: true }),
     }
-    expect(imp.tryThese('foo')).toEqual({ foo: true, p1: true, p2: true })
-    expect(imp.tryThese('foo')).toEqual({ foo: true, p1: true, p2: true })
+    expect(imp.tryTheseOr('foo')).toEqual({ foo: true, p1: true, p2: true })
+    expect(imp.tryTheseOr('foo')).toEqual({ foo: true, p1: true, p2: true })
 
-    expect(imp.tryThese('bar')).toEqual({ bar: true })
+    expect(imp.tryTheseOr('bar')).toEqual({ bar: true })
 
     // ensure cache has been used
     expect(patch1).toHaveBeenCalledTimes(1)

src/util/importer.ts

@@ -1,4 +1,4 @@
-import { ModulePatcher, TBabelCore, TBabelJest, TTypeScript, TsJestImporter } from '../types'
+import { ModulePatcher, TBabelCore, TBabelJest, TTypeScript } from '../types'
 
 import * as hacks from './hacks'
 import { rootLogger } from './logger'
@@ -26,7 +26,7 @@ const passThru = (action: () => void) => (input: any) => {
 /**
  * @internal
  */
-export class Importer implements TsJestImporter {
+export class Importer {
   @Memoize()
   static get instance() {
     logger.debug('creating Importer singleton')
@@ -70,22 +70,51 @@ export class Importer implements TsJestImporter {
   }
 
   @Memoize((...args: string[]) => args.join(':'))
-  tryThese(moduleName: string, ...fallbacks: string[]): any {
+  tryThese(moduleName: string, ...fallbacks: string[]) {
     let name: string
-    let loaded: any
+    let loaded: RequireResult<true> | undefined
     const tries = [moduleName, ...fallbacks]
     // tslint:disable-next-line:no-conditional-assignment
     while ((name = tries.shift() as string) !== undefined) {
-      try {
-        loaded = requireModule(name)
-        logger.debug('loaded module', name)
+      const req = requireWrapper(name)
+
+      // remove exports from what we're going to log
+      const contextReq: any = { ...req }
+      delete contextReq.exports
+
+      if (req.exists) {
+        // module exists
+        loaded = req as RequireResult<true>
+        if (loaded.error) {
+          // require-ing it failed
+          logger.error({ requireResult: contextReq }, `failed loading module '${name}'`, loaded.error.message)
+        } else {
+          // it has been loaded, let's patch it
+          logger.debug({ requireResult: contextReq }, 'loaded module', name)
+          loaded.exports = this._patch(name, loaded.exports)
+        }
         break
-      } catch (err) {
-        logger.debug('fail loading module', name)
+      } else {
+        // module does not exists in the path
+        logger.debug({ requireResult: contextReq }, `module '${name}' not found`)
+        continue
       }
     }
 
-    return loaded && this._patch(name, loaded)
+    // return the loaded one, could be one that has been loaded, or one which has failed during load
+    // but not one which does not exists
+    return loaded
+  }
+
+  tryTheseOr<T>(moduleNames: [string, ...string[]] | string, missingResult: T, allowLoadError?: boolean): T
+  tryTheseOr<T>(moduleNames: [string, ...string[]] | string, missingResult?: T, allowLoadError?: boolean): T | undefined
+  tryTheseOr<T>(moduleNames: [string, ...string[]] | string, missingResult?: T, allowLoadError = false): T | undefined {
+    const args: [string, ...string[]] = Array.isArray(moduleNames) ? moduleNames : [moduleNames]
+    const result = this.tryThese(...args)
+    if (!result) return missingResult
+    if (!result.error) return result.exports as T
+    if (allowLoadError) return missingResult
+    throw result.error
   }
 
   @Memoize(name => name)
@@ -105,8 +134,10 @@ export class Importer implements TsJestImporter {
     // try to load any of the alternative after trying main one
     const res = this.tryThese(moduleName, ...alternatives)
     // if we could load one, return it
-    if (res) {
-      return res
+    if (res && res.exists) {
+      if (!res.error) return res.exports
+      // it could not load because of a failure while importing, but it exists
+      throw new Error(interpolate(Errors.LoadingModuleFailed, { module: res.given, error: res.error.message }))
     }
 
     // if it couldn't load, build a nice error message so the user can fix it by himself
@@ -136,11 +167,43 @@ export class Importer implements TsJestImporter {
  */
 export const importer = Importer.instance
 
+/**
+ * @internal
+ */
+export interface RequireResult<E = boolean> {
+  exists: E
+  given: string
+  path?: string
+  exports?: any
+  error?: Error
+}
+
+function requireWrapper(moduleName: string): RequireResult {
+  let path: string
+  let exists = false
+  try {
+    path = resolveModule(moduleName)
+    exists = true
+  } catch (error) {
+    return { error, exists, given: moduleName }
+  }
+  const result: RequireResult = { exists, path, given: moduleName }
+  try {
+    result.exports = requireModule(moduleName)
+  } catch (error) {
+    result.error = error
+  }
+  return result
+}
+
 let requireModule = (mod: string) => require(mod)
+let resolveModule = (mod: string) => require.resolve(mod)
+
 /**
  * @internal
  */
 // so that we can test easier
-export function __requireModule(localRequire: typeof requireModule) {
+export function __requireModule(localRequire: typeof requireModule, localResolve: typeof resolveModule) {
   requireModule = localRequire
+  resolveModule = localResolve
 }

src/util/messages.ts

@@ -4,6 +4,7 @@
  * @internal
  */
 export enum Errors {
+  LoadingModuleFailed = 'Loading module {{module}} failed with error: {{error}}',
   UnableToLoadOneModule = 'Unable to load the module {{module}}. {{reason}} To fix it:\n{{fix}}',
   UnableToLoadAnyModule = 'Unable to load any of these modules: {{module}}. {{reason}}. To fix it:\n{{fix}}',
   TypesUnavailableWithoutTypeCheck = 'Type information is unavailable with "isolatedModules"',