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 for logging error types instead of strings in the main try/catch #66

Merged
merged 6 commits into from May 12, 2020

Conversation

pjquirk
Copy link
Contributor

@pjquirk pjquirk commented May 12, 2020

When an Error type is thrown instead of a string, the command core.error(error) itself would fail, which would mask the real error. This generally results in an error that is described in actions/toolkit#398.

This bug has been fixed in 1.2.4 of @actions/core, so let's just use that version.

@pjquirk
Copy link
Contributor Author

pjquirk commented May 12, 2020

The failure in test doesn't appear to be new, this PR just makes it be correctly reported as an error now 😄 / 😭

@@ -374,7 +376,12 @@ export class IssueProcessor {
event => event.event === 'labeled' && event.label.name === label
);

return staleLabeledEvent!.created_at;
if (!staleLabeledEvent) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hross FYI

Copy link
Contributor

Choose a reason for hiding this comment

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

awesome. thanks for fixing my bad code :)

@pjquirk pjquirk requested review from hross and ericsciple May 12, 2020 18:48
@@ -374,7 +376,12 @@ export class IssueProcessor {
event => event.event === 'labeled' && event.label.name === label
);

return staleLabeledEvent!.created_at;
if (!staleLabeledEvent) {
Copy link
Contributor

Choose a reason for hiding this comment

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

awesome. thanks for fixing my bad code :)

@hross hross merged commit 71d46bf into actions:master May 12, 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.

None yet

2 participants