Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing error fields when running in jest (26) #422

Closed
Tallyb opened this issue May 7, 2020 · 7 comments · Fixed by #423
Closed

Missing error fields when running in jest (26) #422

Tallyb opened this issue May 7, 2020 · 7 comments · Fixed by #423

Comments

@Tallyb
Copy link

Tallyb commented May 7, 2020

Running execa 4.0.0 with the following code results as follow:

const execa = require('execa');
(async () => {
	// Catching an error
	try {
		let res = (await execa('unknown', ['command']));
	} catch (error) {
		console.log(error);
/*
errno: 'ENOENT',
  code: 'ENOENT',
  syscall: 'spawn unknown',
  path: 'unknown',
  spawnargs: [ 'command' ],
  originalMessage: 'spawn unknown ENOENT',
  shortMessage: 'Command failed with ENOENT: unknown command\nspawn unknown ENOENT',
  command: 'unknown command',
  exitCode: undefined,
  signal: undefined,
  signalDescription: undefined,
  stdout: '',
  stderr: '',
  failed: true,
  timedOut: false,
  isCanceled: false,
  killed: false
  		*/
	}
})();

When running in jest the following code (execa@4 or execa@3):

it ('should have error', async () => {
    try {
        await execa.command('unknown command');
    } catch (e) {
        console.log(`e`);
        expect(e.code).toBeTruthy();
    }
  })
/* {
      "shortMessage": "Command failed with ENOENT: unknown command",
      "command": "unknown command",
      "stdout": "",
      "stderr": "",
      "failed": true,
      "timedOut": false,
      "isCanceled": false,
      "killed": false
    }*/

code and exitCode are missing.

Running the above test with execs@2.0.0 results the following:

{
      "command": "unknown command",
      "exitCode": 2,
      "exitCodeName": "ENOENT",
      "stdout": "",
      "stderr": "",
      "all": "",
      "failed": true,
      "timedOut": false,
      "isCanceled": false,
      "killed": false
    }

I am aware that there are breaking changes between 2 and 3 related to the code, but I am wondering if the exitCode (and code and errno) should remain or not.
Not sure if this is stripped by Jest, but jest is on the same version for both).

@ehmicky
Copy link
Collaborator

ehmicky commented May 7, 2020

Hi @Tallyb, this looks like a bug from Jest, I think this should be reported to them instead?

@Tallyb
Copy link
Author

Tallyb commented May 8, 2020

@ehmicky
As my final note said - if this was really jest, I would expect version 2 to fail as well.
Anyway - it is reported here as well: jestjs/jest#10003

@ehmicky
Copy link
Collaborator

ehmicky commented May 8, 2020

From the answer on the Jest issue, it appears that this is because Execa is using instanceof Error and Jest globals differ from Node.js ones. This seems to me this is something that should be fixed in Jest. What are your thoughts @sindresorhus?

@sindresorhus
Copy link
Owner

Unfortunately, you cannot rely on instanceof for built-ins in JavaScript. It won't work across realms (Node.js vm module, iframes in the browser, etc). I think we'll have to handle it here.

We can do this check instead:

Object.prototype.toString.call(new Error()) === '[object Error]'

@ehmicky
Copy link
Collaborator

ehmicky commented May 8, 2020

Oh I did not know that. So you mean: if a vm module has a different context, but is passed execa from a different context, the Error global would be a different global reference, so instanceof Error would fail?

I noted that the code you posted is what core Node.js seems to do too.

Unfortunately, util.types.isNativeError() does not seem to handle that case.

> Object.prototype.toString.call({ [Symbol.toStringTag]: 'Error' })
'[object Error]'

> util.types.isNativeError({ [Symbol.toStringTag]: 'Error' })
false

Looks like there should be some tiny is-error library to handle this? I could not find one.

@ehmicky
Copy link
Collaborator

ehmicky commented May 8, 2020

PR at #423.

@papb
Copy link
Contributor

papb commented May 8, 2020

Unfortunately, you cannot rely on instanceof for built-ins in JavaScript. It won't work across realms (Node.js vm module, iframes in the browser, etc). I think we'll have to handle it here.

We can do this check instead:

Object.prototype.toString.call(new Error()) === '[object Error]'

Thanks for this! Learning something new every day!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants