Skip to content

Commit 5fa9a85

Browse files
aiqiaoyLouisHaftmann
aiqiaoy
authored and
LouisHaftmann
committed
show helpful error message when resolving actions directly with launch (actions#3874)
1 parent 009956a commit 5fa9a85

File tree

7 files changed

+208
-16
lines changed

7 files changed

+208
-16
lines changed

‎src/Runner.Common/Constants.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@ public static class Features
168168
public static readonly string UseContainerPathForTemplate = "DistributedTask.UseContainerPathForTemplate";
169169
public static readonly string AllowRunnerContainerHooks = "DistributedTask.AllowRunnerContainerHooks";
170170
public static readonly string AddCheckRunIdToJobContext = "actions_add_check_run_id_to_job_context";
171+
public static readonly string DisplayHelpfulActionsDownloadErrors = "actions_display_helpful_actions_download_errors";
171172
}
172173

173174
public static readonly string InternalTelemetryIssueDataKey = "_internal_telemetry";

‎src/Runner.Common/LaunchServer.cs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ public interface ILaunchServer : IRunnerService
1515
{
1616
void InitializeLaunchClient(Uri uri, string token);
1717

18-
Task<ActionDownloadInfoCollection> ResolveActionsDownloadInfoAsync(Guid planId, Guid jobId, ActionReferenceList actionReferenceList, CancellationToken cancellationToken);
18+
Task<ActionDownloadInfoCollection> ResolveActionsDownloadInfoAsync(Guid planId, Guid jobId, ActionReferenceList actionReferenceList, CancellationToken cancellationToken, bool displayHelpfulActionsDownloadErrors);
1919
}
2020

2121
public sealed class LaunchServer : RunnerService, ILaunchServer
@@ -42,12 +42,16 @@ public void InitializeLaunchClient(Uri uri, string token)
4242
}
4343

4444
public Task<ActionDownloadInfoCollection> ResolveActionsDownloadInfoAsync(Guid planId, Guid jobId, ActionReferenceList actionReferenceList,
45-
CancellationToken cancellationToken)
45+
CancellationToken cancellationToken, bool displayHelpfulActionsDownloadErrors)
4646
{
4747
if (_launchClient != null)
4848
{
49-
return _launchClient.GetResolveActionsDownloadInfoAsync(planId, jobId, actionReferenceList,
50-
cancellationToken: cancellationToken);
49+
if (!displayHelpfulActionsDownloadErrors)
50+
{
51+
return _launchClient.GetResolveActionsDownloadInfoAsync(planId, jobId, actionReferenceList,
52+
cancellationToken: cancellationToken);
53+
}
54+
return _launchClient.GetResolveActionsDownloadInfoAsyncV2(planId, jobId, actionReferenceList, cancellationToken);
5155
}
5256

5357
throw new InvalidOperationException("Launch client is not initialized.");

‎src/Runner.Worker/ActionManager.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -688,7 +688,8 @@ private async Task BuildActionContainerAsync(IExecutionContext executionContext,
688688
{
689689
if (MessageUtil.IsRunServiceJob(executionContext.Global.Variables.Get(Constants.Variables.System.JobRequestType)))
690690
{
691-
actionDownloadInfos = await launchServer.ResolveActionsDownloadInfoAsync(executionContext.Global.Plan.PlanId, executionContext.Root.Id, new WebApi.ActionReferenceList { Actions = actionReferences }, executionContext.CancellationToken);
691+
var displayHelpfulActionsDownloadErrors = executionContext.Global.Variables.GetBoolean(Constants.Runner.Features.DisplayHelpfulActionsDownloadErrors) ?? false;
692+
actionDownloadInfos = await launchServer.ResolveActionsDownloadInfoAsync(executionContext.Global.Plan.PlanId, executionContext.Root.Id, new WebApi.ActionReferenceList { Actions = actionReferences }, executionContext.CancellationToken, displayHelpfulActionsDownloadErrors);
692693
}
693694
else
694695
{

‎src/Sdk/WebApi/WebApi/LaunchContracts.cs‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ public class ActionDownloadInfoResponse
2929
{
3030
[DataMember(EmitDefaultValue = false, Name = "authentication")]
3131
public ActionDownloadAuthenticationResponse Authentication { get; set; }
32-
32+
3333
[DataMember(EmitDefaultValue = false, Name = "package_details")]
3434
public ActionDownloadPackageDetailsResponse PackageDetails { get; set; }
3535

@@ -64,7 +64,7 @@ public class ActionDownloadAuthenticationResponse
6464

6565

6666
[DataContract]
67-
public class ActionDownloadPackageDetailsResponse
67+
public class ActionDownloadPackageDetailsResponse
6868
{
6969
[DataMember(EmitDefaultValue = false, Name = "version")]
7070
public string Version { get; set; }
@@ -81,4 +81,25 @@ public class ActionDownloadInfoResponseCollection
8181
[DataMember(EmitDefaultValue = false, Name = "actions")]
8282
public IDictionary<string, ActionDownloadInfoResponse> Actions { get; set; }
8383
}
84+
85+
[DataContract]
86+
public class ActionDownloadResolutionError
87+
{
88+
/// <summary>
89+
/// The error message associated with the action download error.
90+
/// </summary>
91+
[DataMember(EmitDefaultValue = false, Name = "message")]
92+
public string Message { get; set; }
93+
}
94+
95+
[DataContract]
96+
public class ActionDownloadResolutionErrorCollection
97+
{
98+
/// <summary>
99+
/// A mapping of action specifications to their download errors.
100+
/// <remarks>The key is the full name of the action plus version, e.g. "actions/checkout@v2".</remarks>
101+
/// </summary>
102+
[DataMember(EmitDefaultValue = false, Name = "errors")]
103+
public IDictionary<string, ActionDownloadResolutionError> Errors { get; set; }
104+
}
84105
}

‎src/Sdk/WebApi/WebApi/LaunchHttpClient.cs‎

Lines changed: 46 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
using System;
44
using System.Linq;
5+
using System.Net;
56
using System.Net.Http;
67
using System.Net.Http.Formatting;
78
using System.Net.Http.Headers;
@@ -32,11 +33,52 @@ public LaunchHttpClient(
3233
public async Task<ActionDownloadInfoCollection> GetResolveActionsDownloadInfoAsync(Guid planId, Guid jobId, ActionReferenceList actionReferenceList, CancellationToken cancellationToken)
3334
{
3435
var GetResolveActionsDownloadInfoURLEndpoint = new Uri(m_launchServiceUrl, $"/actions/build/{planId.ToString()}/jobs/{jobId.ToString()}/runnerresolve/actions");
35-
return ToServerData(await GetLaunchSignedURLResponse<ActionReferenceRequestList, ActionDownloadInfoResponseCollection>(GetResolveActionsDownloadInfoURLEndpoint, ToGitHubData(actionReferenceList), cancellationToken));
36+
var response = await GetLaunchSignedURLResponse<ActionReferenceRequestList>(GetResolveActionsDownloadInfoURLEndpoint, ToGitHubData(actionReferenceList), cancellationToken);
37+
return ToServerData(await ReadJsonContentAsync<ActionDownloadInfoResponseCollection>(response, cancellationToken));
3638
}
3739

38-
// Resolve Actions
39-
private async Task<T> GetLaunchSignedURLResponse<R, T>(Uri uri, R request, CancellationToken cancellationToken)
40+
public async Task<ActionDownloadInfoCollection> GetResolveActionsDownloadInfoAsyncV2(Guid planId, Guid jobId, ActionReferenceList actionReferenceList, CancellationToken cancellationToken)
41+
{
42+
var GetResolveActionsDownloadInfoURLEndpoint = new Uri(m_launchServiceUrl, $"/actions/build/{planId.ToString()}/jobs/{jobId.ToString()}/runnerresolve/actions");
43+
var response = await GetLaunchSignedURLResponse<ActionReferenceRequestList>(GetResolveActionsDownloadInfoURLEndpoint, ToGitHubData(actionReferenceList), cancellationToken);
44+
45+
if (response.IsSuccessStatusCode)
46+
{
47+
// Success response - deserialize the action download info
48+
return ToServerData(await ReadJsonContentAsync<ActionDownloadInfoResponseCollection>(response, cancellationToken));
49+
}
50+
51+
var responseError = response.ReasonPhrase ?? "";
52+
if (response.StatusCode == HttpStatusCode.UnprocessableEntity)
53+
{
54+
// 422 response - unresolvable actions, error details are in the body
55+
var errors = await ReadJsonContentAsync<ActionDownloadResolutionErrorCollection>(response, cancellationToken);
56+
string combinedErrorMessage;
57+
if (errors?.Errors != null && errors.Errors.Any())
58+
{
59+
combinedErrorMessage = String.Join(". ", errors.Errors.Select(kvp => kvp.Value.Message));
60+
}
61+
else
62+
{
63+
combinedErrorMessage = responseError;
64+
}
65+
66+
throw new UnresolvableActionDownloadInfoException(combinedErrorMessage);
67+
}
68+
else if (response.StatusCode == HttpStatusCode.TooManyRequests)
69+
{
70+
// Here we want to add a message so customers don't think it's a rate limit scoped to them
71+
// Ideally this would be 500 but the runner retries 500s, which we don't want to do when we're being rate limited
72+
// See: https://github.com/github/ecosystem-api/issues/4084
73+
throw new NonRetryableActionDownloadInfoException(responseError + " (GitHub has reached an internal rate limit, please try again later)");
74+
}
75+
else
76+
{
77+
throw new Exception(responseError);
78+
}
79+
}
80+
81+
private async Task<HttpResponseMessage> GetLaunchSignedURLResponse<R>(Uri uri, R request, CancellationToken cancellationToken)
4082
{
4183
using (HttpRequestMessage requestMessage = new HttpRequestMessage(HttpMethod.Post, uri))
4284
{
@@ -46,10 +88,7 @@ private async Task<T> GetLaunchSignedURLResponse<R, T>(Uri uri, R request, Cance
4688
using (HttpContent content = new ObjectContent<R>(request, m_formatter))
4789
{
4890
requestMessage.Content = content;
49-
using (var response = await SendAsync(requestMessage, HttpCompletionOption.ResponseContentRead, cancellationToken: cancellationToken))
50-
{
51-
return await ReadJsonContentAsync<T>(response, cancellationToken);
52-
}
91+
return await SendAsync(requestMessage, HttpCompletionOption.ResponseContentRead, cancellationToken: cancellationToken);
5392
}
5493
}
5594
}

‎src/Test/L0/Sdk/LaunchWebApi/LaunchHttpClientL0.cs‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
using GitHub.Actions.RunService.WebApi;
2+
using GitHub.DistributedTask.WebApi;
3+
using GitHub.Services.Launch.Client;
4+
using GitHub.Services.Launch.Contracts;
5+
using Moq;
6+
using Moq.Protected;
7+
using System;
8+
using System.Collections.Generic;
9+
using System.Linq;
10+
using System.Net;
11+
using System.Net.Http;
12+
using System.Text;
13+
using System.Threading;
14+
using System.Threading.Tasks;
15+
using Xunit;
16+
17+
namespace GitHub.Actions.RunService.WebApi.Tests
18+
{
19+
public sealed class LaunchHttpClientL0
20+
{
21+
[Fact]
22+
public async Task GetResolveActionsDownloadInfoAsync_SuccessResponse()
23+
{
24+
var baseUrl = new Uri("https://api.github.com/");
25+
var planId = Guid.NewGuid();
26+
var jobId = Guid.NewGuid();
27+
var token = "fake-token";
28+
29+
var actionReferenceList = new ActionReferenceList
30+
{
31+
Actions = new List<ActionReference>
32+
{
33+
new ActionReference
34+
{
35+
NameWithOwner = "owner1/action1",
36+
Ref = "0123456789"
37+
}
38+
}
39+
};
40+
41+
var responseContent = @"{
42+
""actions"": {
43+
""owner1/action1@0123456789"": {
44+
""name"": ""owner1/action1"",
45+
""resolved_name"": ""owner1/action1"",
46+
""resolved_sha"": ""0123456789"",
47+
""version"": ""0123456789"",
48+
""zip_url"": ""https://github.com/owner1/action1/zip"",
49+
""tar_url"": ""https://github.com/owner1/action1/tar""
50+
}
51+
}
52+
}";
53+
54+
var httpResponse = new HttpResponseMessage(HttpStatusCode.OK)
55+
{
56+
Content = new StringContent(responseContent, Encoding.UTF8, "application/json"),
57+
RequestMessage = new HttpRequestMessage()
58+
{
59+
RequestUri = new Uri($"{baseUrl}actions/build/{planId}/jobs/{jobId}/runnerresolve/actions")
60+
}
61+
};
62+
63+
var mockHandler = new Mock<HttpMessageHandler>();
64+
mockHandler.Protected().Setup<Task<HttpResponseMessage>>("SendAsync", ItExpr.IsAny<HttpRequestMessage>(), ItExpr.IsAny<CancellationToken>())
65+
.ReturnsAsync(httpResponse);
66+
67+
var client = new LaunchHttpClient(baseUrl, mockHandler.Object, token, false);
68+
var result = await client.GetResolveActionsDownloadInfoAsyncV2(planId, jobId, actionReferenceList, CancellationToken.None);
69+
70+
// Assert
71+
Assert.NotNull(result);
72+
Assert.NotEmpty(result.Actions);
73+
Assert.Equal(actionReferenceList.Actions.Count, result.Actions.Count);
74+
Assert.True(result.Actions.ContainsKey("owner1/action1@0123456789"));
75+
}
76+
77+
[Fact]
78+
public async Task GetResolveActionsDownloadInfoAsync_UnprocessableEntityResponse()
79+
{
80+
var baseUrl = new Uri("https://api.github.com/");
81+
var planId = Guid.NewGuid();
82+
var jobId = Guid.NewGuid();
83+
var token = "fake-token";
84+
85+
var actionReferenceList = new ActionReferenceList
86+
{
87+
Actions = new List<ActionReference>
88+
{
89+
new ActionReference
90+
{
91+
NameWithOwner = "owner1/action1",
92+
Ref = "0123456789"
93+
}
94+
}
95+
};
96+
97+
var responseContent = @"{
98+
""errors"": {
99+
""owner1/invalid-action@0123456789"": {
100+
""message"": ""Unable to resolve action 'owner1/invalid-action@0123456789', repository not found""
101+
}
102+
}
103+
}";
104+
105+
var httpResponse = new HttpResponseMessage(HttpStatusCode.UnprocessableEntity)
106+
{
107+
Content = new StringContent(responseContent, Encoding.UTF8, "application/json"),
108+
RequestMessage = new HttpRequestMessage()
109+
{
110+
RequestUri = new Uri($"{baseUrl}actions/build/{planId}/jobs/{jobId}/runnerresolve/actions")
111+
}
112+
};
113+
114+
var mockHandler = new Mock<HttpMessageHandler>();
115+
mockHandler.Protected().Setup<Task<HttpResponseMessage>>("SendAsync", ItExpr.IsAny<HttpRequestMessage>(), ItExpr.IsAny<CancellationToken>())
116+
.ReturnsAsync(httpResponse);
117+
118+
var client = new LaunchHttpClient(baseUrl, mockHandler.Object, token, false);
119+
120+
var exception = await Assert.ThrowsAsync<UnresolvableActionDownloadInfoException>(
121+
() => client.GetResolveActionsDownloadInfoAsyncV2(planId, jobId, actionReferenceList, CancellationToken.None));
122+
123+
Assert.Contains("repository not found", exception.Message);
124+
}
125+
}
126+
}

‎src/Test/L0/Worker/ActionManagerL0.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2411,8 +2411,8 @@ private void Setup([CallerMemberName] string name = "", bool enableComposite = t
24112411
});
24122412

24132413
_launchServer = new Mock<ILaunchServer>();
2414-
_launchServer.Setup(x => x.ResolveActionsDownloadInfoAsync(It.IsAny<Guid>(), It.IsAny<Guid>(), It.IsAny<ActionReferenceList>(), It.IsAny<CancellationToken>()))
2415-
.Returns((Guid planId, Guid jobId, ActionReferenceList actions, CancellationToken cancellationToken) =>
2414+
_launchServer.Setup(x => x.ResolveActionsDownloadInfoAsync(It.IsAny<Guid>(), It.IsAny<Guid>(), It.IsAny<ActionReferenceList>(), It.IsAny<CancellationToken>(), It.IsAny<bool>()))
2415+
.Returns((Guid planId, Guid jobId, ActionReferenceList actions, CancellationToken cancellationToken, bool displayHelpfulActionsDownloadErrors) =>
24162416
{
24172417
var result = new ActionDownloadInfoCollection { Actions = new Dictionary<string, ActionDownloadInfo>() };
24182418
foreach (var action in actions.Actions)

0 commit comments

Comments
 (0)