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.

Sign up for GitHub

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

Jump to bottom

Write error information to STDERR #4799

Merged
merged 3 commits into from Jun 3, 2020
Merged

Write error information to STDERR #4799

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9967934
Change to standard error
naokikimura May 21, 2020
e032125
Fix a test
naokikimura May 21, 2020
fe306da
Use process.stderr.write instead of console.error
m-allanson Jun 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 7 additions & 3 deletions lib/__tests__/cli.test.js
View file
Open in desktop
Expand Up @@ -159,7 +159,9 @@ describe('CLI', () => {
beforeAll(() => {
jest.spyOn(process, 'exit').mockImplementation(() => {});
jest.spyOn(process.stdout, 'write').mockImplementation(() => {});
jest.spyOn(process.stderr, 'write').mockImplementation(() => {});
jest.spyOn(console, 'log').mockImplementation(() => {});
jest.spyOn(console, 'error').mockImplementation(() => {});
});

afterAll(() => {
Expand Down Expand Up @@ -259,9 +261,11 @@ describe('CLI', () => {
replaceBackslashes(path.join(fixturesPath, 'empty-block-with-disables.css')),
]);

expect(process.exit).not.toHaveBeenCalledWith(0);
expect(process.exitCode).toEqual(1);

expect(console.log).toHaveBeenCalledTimes(1);
expect(console.log).toHaveBeenCalledWith(expect.stringContaining('Cannot find module'));
expect(process.stderr.write).toHaveBeenCalledTimes(1);
expect(process.stderr.write).toHaveBeenCalledWith(
expect.stringContaining('Cannot find module'),
);
});
});
4 changes: 2 additions & 2 deletions lib/cli.js
View file
Open in desktop
Expand Up @@ -534,10 +534,10 @@ module.exports = (argv) => {
* @returns {void}
*/
function handleError(err) {
console.log(err.stack); // eslint-disable-line no-console
process.stderr.write(err.stack);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've changed this from console.error to process.stderr.write. This is consistent with other errors earlier in the file, and means we can remove the // eslint-disable-line comment.

const exitCode = typeof err.code === 'number' ? err.code : 1;

process.exit(exitCode); // eslint-disable-line no-process-exit
process.exitCode = exitCode;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set an exitCode and wait for node to exit when any pending work is complete. This means we can remove another // eslint-disable-line comment. The node docs recommend against using process.exit() where possible.

}

/**
Expand Down