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: delete execa std props #314

Merged
merged 1 commit into from
Jun 12, 2024
Merged

fix: delete execa std props #314

merged 1 commit into from
Jun 12, 2024

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented Jun 12, 2024

Example: https://github.com/vitejs/vite-ecosystem-ci/actions/runs/9476969819/job/26110768545

The execa std errors are hard to read, especially that we had already piped the output the the main process, so I don't think we need the props anymore.

Comment on lines +55 to +61
} catch (error) {
// Since we already piped the io to the parent process, we remove the duplicated
// messages here so it's easier to read the error message.
if (error.stdout) error.stdout = 'value removed by vite-ecosystem-ci'
if (error.stderr) error.stderr = 'value removed by vite-ecosystem-ci'
if (error.stdio) error.stdio = ['value removed by vite-ecosystem-ci']
throw error
Copy link
Member Author

Choose a reason for hiding this comment

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

It's also possible to check with error instanceof ExecaError, but for some reason the types are different, e.g. it marked error.stdout: undefined, which is not true.

@dominikg dominikg merged commit b701fa6 into main Jun 12, 2024
1 check failed
@dominikg dominikg deleted the execa-clean-error branch June 12, 2024 20:59
haoqunjiang pushed a commit to vuejs/ecosystem-ci that referenced this pull request Jun 14, 2024
danielroe pushed a commit to nuxt/ecosystem-ci that referenced this pull request Aug 13, 2024
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.

3 participants