fix: don't assume stack is always a string (#10697)

Author: snitin315

CHANGELOG.md

@@ -8,6 +8,7 @@
 
 - `[jest-config]` Fix bug introduced in watch mode by PR[#10678](https://github.com/facebook/jest/pull/10678/files#r511037803) ([#10692](https://github.com/facebook/jest/pull/10692))
 - `[expect]` Stop modifying the sample in `expect.objectContaining()` ([#10711](https://github.com/facebook/jest/pull/10711))
+- `[jest-circus, jest-jasmine2]` fix: don't assume `stack` is always a string ([#10697](https://github.com/facebook/jest/pull/10697))
 - `[jest-resolve-dependencies]` Resolve mocks as dependencies ([#10713](https://github.com/facebook/jest/pull/10713))
 
 ### Chore & Maintenance

e2e/__tests__/__snapshots__/failures.test.ts.snap

@@ -18,7 +18,7 @@ exports[`not throwing Error objects 3`] = `
 FAIL __tests__/throwObject.test.js
   ● Test suite failed to run
 
-    Error: No message was provided
+    thrown: Object {}
 `;
 
 exports[`not throwing Error objects 4`] = `
@@ -117,6 +117,7 @@ FAIL __tests__/duringTests.test.js
   ✕ Boolean thrown during test
   ✕ undefined thrown during test
   ✕ Object thrown during test
+  ✕ Object with stack prop thrown during test
   ✕ Error during test
   ✕ done(Error)
   ✕ done(non-error)
@@ -185,33 +186,49 @@ FAIL __tests__/duringTests.test.js
 
       at Object.test (__tests__/duringTests.test.js:28:1)
 
+  ● Object with stack prop thrown during test
+
+    thrown: Object {
+      "stack": 42,
+    }
+
+      30 | });
+      31 | 
+    > 32 | test('Object with stack prop thrown during test', () => {
+         | ^
+      33 |   // eslint-disable-next-line no-throw-literal
+      34 |   throw {stack: 42};
+      35 | });
+
+      at Object.test (__tests__/duringTests.test.js:32:1)
+
   ● Error during test
 
     ReferenceError: doesNotExist is not defined
 
-      32 | test('Error during test', () => {
-      33 |   // eslint-disable-next-line no-undef
-    > 34 |   doesNotExist.alsoThisNot;
+      37 | test('Error during test', () => {
+      38 |   // eslint-disable-next-line no-undef
+    > 39 |   doesNotExist.alsoThisNot;
          |   ^
-      35 | });
-      36 | 
-      37 | test('done(Error)', done => {
+      40 | });
+      41 | 
+      42 | test('done(Error)', done => {
 
-      at Object.doesNotExist (__tests__/duringTests.test.js:34:3)
+      at Object.doesNotExist (__tests__/duringTests.test.js:39:3)
 
   ● done(Error)
 
     this is an error
 
-      36 | 
-      37 | test('done(Error)', done => {
-    > 38 |   done(new Error('this is an error'));
+      41 | 
+      42 | test('done(Error)', done => {
+    > 43 |   done(new Error('this is an error'));
          |        ^
-      39 | });
-      40 | 
-      41 | test('done(non-error)', done => {
+      44 | });
+      45 | 
+      46 | test('done(non-error)', done => {
 
-      at Object.<anonymous> (__tests__/duringTests.test.js:38:8)
+      at Object.<anonymous> (__tests__/duringTests.test.js:43:8)
 
   ● done(non-error)
 
@@ -224,15 +241,15 @@ FAIL __tests__/duringTests.test.js
       ],
     }
 
-      40 | 
-      41 | test('done(non-error)', done => {
-    > 42 |   done(deepObject);
+      45 | 
+      46 | test('done(non-error)', done => {
+    > 47 |   done(deepObject);
          |   ^
-      43 | });
-      44 | 
-      45 | test('returned promise rejection', () => Promise.reject(deepObject));
+      48 | });
+      49 | 
+      50 | test('returned promise rejection', () => Promise.reject(deepObject));
 
-      at Object.done (__tests__/duringTests.test.js:42:3)
+      at Object.done (__tests__/duringTests.test.js:47:3)
 
   ● returned promise rejection
 
@@ -245,13 +262,23 @@ FAIL __tests__/duringTests.test.js
       ],
     }
 
-      43 | });
-      44 | 
-    > 45 | test('returned promise rejection', () => Promise.reject(deepObject));
+      48 | });
+      49 | 
+    > 50 | test('returned promise rejection', () => Promise.reject(deepObject));
          | ^
-      46 | 
+      51 | 
 
-      at Object.test (__tests__/duringTests.test.js:45:1)
+      at Object.test (__tests__/duringTests.test.js:50:1)
+`;
+
+exports[`not throwing Error objects 6`] = `
+FAIL __tests__/throwObjectWithStackProp.test.js
+  ● Test suite failed to run
+
+    thrown: Object {
+
+        "stack": 42,
+      }
 `;
 
 exports[`works with assertions in separate files 1`] = `

e2e/__tests__/failures.test.ts

@@ -40,7 +40,7 @@ test('not throwing Error objects', () => {
   stderr = runJest(dir, ['duringTests.test.js']).stderr;
 
   if (nodeMajorVersion < 12) {
-    const lineEntry = '(__tests__/duringTests.test.js:38:8)';
+    const lineEntry = '(__tests__/duringTests.test.js:43:8)';
 
     expect(stderr).toContain(`at Object.<anonymous>.done ${lineEntry}`);
 
@@ -51,6 +51,8 @@ test('not throwing Error objects', () => {
   }
 
   expect(wrap(cleanStderr(stderr))).toMatchSnapshot();
+  stderr = runJest(dir, ['throwObjectWithStackProp.test.js']).stderr;
+  expect(wrap(cleanStderr(stderr))).toMatchSnapshot();
 });
 
 test('works with node assert', () => {

e2e/failures/__tests__/duringTests.test.js

@@ -29,6 +29,11 @@ test('Object thrown during test', () => {
   throw deepObject;
 });
 
+test('Object with stack prop thrown during test', () => {
+  // eslint-disable-next-line no-throw-literal
+  throw {stack: 42};
+});
+
 test('Error during test', () => {
   // eslint-disable-next-line no-undef
   doesNotExist.alsoThisNot;

e2e/failures/__tests__/throwObjectWithStackProp.test.js

@@ -0,0 +1,11 @@
+/**
+ * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+'use strict';
+
+// eslint-disable-next-line no-throw-literal
+throw {stack: 42};

packages/jest-circus/src/utils.ts

@@ -383,7 +383,7 @@ const _getError = (
     asyncError = new Error();
   }
 
-  if (error && (error.stack || error.message)) {
+  if (error && (typeof error.stack === 'string' || error.message)) {
     return error;
   }
 
@@ -392,7 +392,8 @@ const _getError = (
   return asyncError;
 };
 
-const getErrorStack = (error: Error): string => error.stack || error.message;
+const getErrorStack = (error: Error): string =>
+  typeof error.stack === 'string' ? error.stack : error.message;
 
 export const addErrorToEachTestUnderDescribe = (
   describeBlock: Circus.DescribeBlock,

packages/jest-jasmine2/src/__tests__/__snapshots__/expectationResultFactory.test.ts.snap

@@ -1,5 +1,11 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`expectationResultFactory returns the result if failed (with \`error.stack\` not as a string). 1`] = `
+"thrown: Object {
+  \\"stack\\": 42,
+}"
+`;
+
 exports[`expectationResultFactory returns the result if passed. 1`] = `
 Object {
   "error": undefined,

packages/jest-jasmine2/src/__tests__/expectationResultFactory.test.ts

@@ -78,4 +78,16 @@ describe('expectationResultFactory', () => {
     const result = expectationResultFactory(options);
     expect(result.message).toEqual('Expected `Pass`, received `Fail`.');
   });
+
+  it('returns the result if failed (with `error.stack` not as a string).', () => {
+    const options = {
+      actual: 'Fail',
+      error: {stack: 42},
+      expected: 'Pass',
+      matcherName: 'testMatcher',
+      passed: false,
+    };
+    const result = expectationResultFactory(options);
+    expect(result.message).toMatchSnapshot();
+  });
 });

packages/jest-jasmine2/src/expectationResultFactory.ts

@@ -43,7 +43,7 @@ function stackFormatter(
   }
 
   if (options.error) {
-    if (options.error.stack) {
+    if (typeof options.error.stack === 'string') {
       return options.error.stack;
     }
 

packages/jest-jasmine2/src/reporter.ts

@@ -152,7 +152,7 @@ export default class Jasmine2Reporter implements Reporter {
 
     specResult.failedExpectations.forEach(failed => {
       const message =
-        !failed.matcherName && failed.stack
+        !failed.matcherName && typeof failed.stack === 'string'
           ? this._addMissingMessageToStack(failed.stack, failed.message)
           : failed.message || '';
       results.failureMessages.push(message);

packages/jest-message-util/package.json

@@ -19,6 +19,7 @@
     "chalk": "^4.0.0",
     "graceful-fs": "^4.2.4",
     "micromatch": "^4.0.2",
+    "pretty-format": "^26.6.1",
     "slash": "^3.0.0",
     "stack-utils": "^2.0.2"
   },

packages/jest-message-util/src/index.ts

@@ -10,6 +10,7 @@ import * as fs from 'graceful-fs';
 import type {Config, TestResult} from '@jest/types';
 import chalk = require('chalk');
 import micromatch = require('micromatch');
+import prettyFormat = require('pretty-format');
 import slash = require('slash');
 import {codeFrameColumns} from '@babel/code-frame';
 import StackUtils = require('stack-utils');
@@ -140,7 +141,10 @@ export const formatExecError = (
     stack = error;
   } else {
     message = error.message;
-    stack = error.stack;
+    stack =
+      typeof error.stack === 'string'
+        ? error.stack
+        : `thrown: ${prettyFormat(error, {maxDepth: 3})}`;
   }
 
   const separated = separateMessageFromStack(stack || '');
@@ -160,9 +164,12 @@ export const formatExecError = (
       ? '\n' + formatStackTrace(stack, config, options, testPath)
       : '';
 
-  if (blankStringRegexp.test(message) && blankStringRegexp.test(stack)) {
+  if (
+    typeof stack !== 'string' ||
+    (blankStringRegexp.test(message) && blankStringRegexp.test(stack))
+  ) {
     // this can happen if an empty object is thrown.
-    message = MESSAGE_INDENT + 'Error: No message was provided';
+    message = `thrown: ${prettyFormat(error, {maxDepth: 3})}`;
   }
 
   let messageToUse;

packages/jest-message-util/tsconfig.json

@@ -4,5 +4,8 @@
     "rootDir": "src",
     "outDir": "build"
   },
-  "references": [{"path": "../jest-types"}]
+  "references": [
+    {"path": "../jest-types"},
+    {"path": "../pretty-format"}
+  ]
 }

yarn.lock

@@ -11728,6 +11728,7 @@ fsevents@^1.2.7:
     chalk: ^4.0.0
     graceful-fs: ^4.2.4
     micromatch: ^4.0.2
+    pretty-format: ^26.6.1
     slash: ^3.0.0
     stack-utils: ^2.0.2
   languageName: unknown