Skip to content

Conversation

@vmjoseph
Copy link
Contributor

@vmjoseph vmjoseph commented Mar 13, 2023

Fix for https://github.com/github/c2c-actions-runtime/issues/2182:

When an action fails to download, it does not get consistently flagged in Kusto as an infrastructure error. After manually throwing an error in the download function (GetDownloadInfoAsync), we should bubble up the error so that it can be caught by the original caller.

To replicate locally:

  • Add throw new WebApi.FailedToResolveActionDownloadInfoException(...) here
  • Configure, build, and run as usual.

Example Repo with error flagging:
https://github.com/vmjoseph/CompositeActionsTest/actions/runs/4546302712/jobs/8014764926

@vmjoseph vmjoseph marked this pull request as ready for review March 17, 2023 15:03
@vmjoseph vmjoseph requested a review from a team as a code owner March 17, 2023 15:03
@vmjoseph vmjoseph changed the title WIP Composite Action Incidents Composite Action Incidents Mar 20, 2023
@vmjoseph vmjoseph changed the title Composite Action Incidents Adding Consistency to 'Failed To Resolve Action Download Info` Infrastructure Error Flagging Mar 28, 2023
@vmjoseph vmjoseph changed the title Adding Consistency to 'Failed To Resolve Action Download Info` Infrastructure Error Flagging Adding Consistency to 'Failed To Resolve Action Download Info' Infrastructure Error Flagging Mar 28, 2023
}
catch (WebApi.FailedToResolveActionDownloadInfoException ex)
{
throw new WebApi.FailedToResolveActionDownloadInfoException("Failed to resolve action download info", ex);
Copy link
Member

Choose a reason for hiding this comment

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

why do we need to catch the exception and rethrow the same exception with a different message?

@vmjoseph vmjoseph merged commit 49b0497 into main May 11, 2023
@vmjoseph vmjoseph deleted the vmjoseph/slo-incident branch May 11, 2023 18:57
nikola-jokic pushed a commit to nikola-jokic/runner that referenced this pull request May 12, 2023
…tructure Error Flagging (actions#2488)

* adding extra catch for download failure in composite actions

* Adding infra error

* Adding error handling centralizing

* updating try catch bubbling

* cleaning up commits

* cleaning up commits

* cleaning up commits

* updating bubbler

* cleaning up test files

* Fixing linting errors

* updating exception bubble

* reverting composite

* updating catch to not exclude other exceptions

* removing uneeded import

* Update src/Runner.Worker/ActionRunner.cs

Co-authored-by: Tingluo Huang <[email protected]>

* Update src/Runner.Worker/ActionManager.cs

Co-authored-by: Tingluo Huang <[email protected]>

* Update src/Runner.Worker/ActionManager.cs

Co-authored-by: Tingluo Huang <[email protected]>

* Update src/Runner.Worker/ActionManager.cs

Co-authored-by: Tingluo Huang <[email protected]>

* Update src/Runner.Worker/ActionManager.cs

Co-authored-by: Tingluo Huang <[email protected]>

* moving download out of for loop; reverting exception wrap

* Update src/Runner.Worker/ActionManager.cs

Co-authored-by: Tingluo Huang <[email protected]>

* Adding blank lines back

* Adding blank lines back

* removing uneeded catch for download fail

* adding var back for consistency

* formatting clean

---------

Co-authored-by: Tingluo Huang <[email protected]>
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.

5 participants