Skip to content

Conversation

@robertbrignull
Copy link
Contributor

Doing core.setFailed(e.message); is good as it highlights the error message to the user. It's unfortunately less useful to us for debugging because it doesn't include the stack trace. This PR amends every place to do core.setFailed to also log the full error to the console.

I picked console.log over core.info as that's what we currently do at the action entrypoints (e.g.

run().catch(e => {
  core.setFailed("init action failed: " + e);
  console.log(e);
});

) but we could change those to be core.info instead. I'm pretty ambivalent.

This could be another candidate for a custom CodeQL query for this repo, to catch places where we aren't logging the full error.

Here's what it looks like: https://github.com/dsp-testing/robertbrignull_no_language/runs/948487952?check_suite_focus=true

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.

Copy link
Contributor

@aeisenberg aeisenberg left a comment

Choose a reason for hiding this comment

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

That looks good to me. Surprised there is't a core function that specifically logs errors.

@robertbrignull robertbrignull merged commit 8608105 into main Aug 6, 2020
@robertbrignull robertbrignull deleted the always_log_error branch August 6, 2020 16:33
@github-actions github-actions bot mentioned this pull request Aug 10, 2020
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