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

fix: don't log stacktrace if cli succeeds (regression) #182

Merged
merged 8 commits into from Mar 15, 2023

Conversation

tommy-mitchell
Copy link
Contributor

@tommy-mitchell tommy-mitchell commented Mar 14, 2023

Starting in v0.26.1, successful runs of the CLI (i.e. when it doesn't crash) incorrectly report an error and error stack while running tsd:

Error running tsd: Error: 
  index.test-d.ts:26:0
  ✖  25:0  Expected an error, but found none.  
  ✖  26:0  ) expected.                         

  2 errors

    at ~/node_modules/tsd/dist/cli.js:64:19
    at Generator.next (<anonymous>)
    at fulfilled (~/node_modules/tsd/dist/cli.js:6:58)

This PR restores the old behavior and makes sure that the error message and stack are only displayed when tsd crashes.

@tommy-mitchell
Copy link
Contributor Author

I’m not sure why the tests are failing in Node 14.

@tommy-mitchell
Copy link
Contributor Author

Turns out error.cause isn't supported in Node 14:

throw new Error(formatter(diagnostics, showDiff), {cause: 'tsd found diagnostics'});

source/test/cli.ts Outdated Show resolved Hide resolved
source/cli.ts Outdated Show resolved Hide resolved
source/cli.ts Outdated Show resolved Hide resolved
source/cli.ts Outdated Show resolved Hide resolved
@tommy-mitchell
Copy link
Contributor Author

It now only throws on failure, and otherwise exits with an error code if any error-level diagnostics are found:

const exit = (message: string, {isError = true}: {isError?: boolean} = {}) => {
	if (isError) {
		console.error(message);
		process.exit(1);
	} else {
		console.log(message);
		process.exit(0);
	}
};

try {
	// ...
	if (diagnostics.length > 0) {
		const hasErrors = diagnostics.some(diagnostic => diagnostic.severity === 'error');
		const formattedDiagnostics = formatter(diagnostics, showDiff);

		exit(formattedDiagnostics, {isError: hasErrors});
	}
} catch (error: unknown) {
	const potentialError = error as Error | undefined;
	const errorMessage = potentialError?.stack ?? potentialError?.message ?? 'tsd unexpectedly crashed.';

	exit(`Error running tsd:\n${errorMessage}`);
}

@tommy-mitchell
Copy link
Contributor Author

tommy-mitchell commented Mar 14, 2023

The formatter also needs to report the number of warnings, but I have another PR for that.

Edit: #184

@sindresorhus sindresorhus merged commit 2dc5358 into tsdjs:main Mar 15, 2023
3 checks passed
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 this pull request may close these issues.

None yet

2 participants