From 4c0a43f0e48b21fc23bf7458cd5657863a7a80db Mon Sep 17 00:00:00 2001 From: Luke Tomlinson Date: Thu, 5 Sep 2024 17:08:57 -0400 Subject: [PATCH] Handle Error Body in Responses from Broker (#3454) --- src/Runner.Common/BrokerServer.cs | 2 +- src/Sdk/RSWebApi/Contracts/BrokerError.cs | 20 +++++++++ src/Sdk/RSWebApi/Contracts/BrokerErrorKind.cs | 10 +++++ src/Sdk/WebApi/WebApi/BrokerHttpClient.cs | 41 +++++++++++++++++-- 4 files changed, 69 insertions(+), 4 deletions(-) create mode 100644 src/Sdk/RSWebApi/Contracts/BrokerError.cs create mode 100644 src/Sdk/RSWebApi/Contracts/BrokerErrorKind.cs diff --git a/src/Runner.Common/BrokerServer.cs b/src/Runner.Common/BrokerServer.cs index 5e1311715..4c612e961 100644 --- a/src/Runner.Common/BrokerServer.cs +++ b/src/Runner.Common/BrokerServer.cs @@ -92,7 +92,7 @@ namespace GitHub.Runner.Common public bool ShouldRetryException(Exception ex) { - if (ex is AccessDeniedException ade && ade.ErrorCode == 1) + if (ex is AccessDeniedException ade) { return false; } diff --git a/src/Sdk/RSWebApi/Contracts/BrokerError.cs b/src/Sdk/RSWebApi/Contracts/BrokerError.cs new file mode 100644 index 000000000..c2e4bfa7b --- /dev/null +++ b/src/Sdk/RSWebApi/Contracts/BrokerError.cs @@ -0,0 +1,20 @@ +using System.Runtime.Serialization; + +namespace GitHub.Actions.RunService.WebApi +{ + [DataContract] + public class BrokerError + { + [DataMember(Name = "source", EmitDefaultValue = false)] + public string Source { get; set; } + + [DataMember(Name = "errorKind", EmitDefaultValue = false)] + public string ErrorKind { get; set; } + + [DataMember(Name = "statusCode", EmitDefaultValue = false)] + public int StatusCode { get; set; } + + [DataMember(Name = "errorMessage", EmitDefaultValue = false)] + public string Message { get; set; } + } +} diff --git a/src/Sdk/RSWebApi/Contracts/BrokerErrorKind.cs b/src/Sdk/RSWebApi/Contracts/BrokerErrorKind.cs new file mode 100644 index 000000000..15c423017 --- /dev/null +++ b/src/Sdk/RSWebApi/Contracts/BrokerErrorKind.cs @@ -0,0 +1,10 @@ +using System.Runtime.Serialization; + +namespace GitHub.Actions.RunService.WebApi +{ + [DataContract] + public class BrokerErrorKind + { + public const string RunnerVersionTooOld = "RunnerVersionTooOld"; + } +} diff --git a/src/Sdk/WebApi/WebApi/BrokerHttpClient.cs b/src/Sdk/WebApi/WebApi/BrokerHttpClient.cs index e9ad938fb..8b67da2e5 100644 --- a/src/Sdk/WebApi/WebApi/BrokerHttpClient.cs +++ b/src/Sdk/WebApi/WebApi/BrokerHttpClient.cs @@ -103,6 +103,7 @@ namespace GitHub.Actions.RunService.WebApi new HttpMethod("GET"), requestUri: requestUri, queryParameters: queryParams, + readErrorBody: true, cancellationToken: cancellationToken); if (result.IsSuccess) @@ -110,8 +111,21 @@ namespace GitHub.Actions.RunService.WebApi return result.Value; } - // the only time we throw a `Forbidden` exception from Listener /messages is when the runner is - // disable_update and is too old to poll + if (TryParseErrorBody(result.ErrorBody, out BrokerError brokerError)) + { + switch (brokerError.ErrorKind) + { + case BrokerErrorKind.RunnerVersionTooOld: + throw new AccessDeniedException(brokerError.Message) + { + ErrorCode = 1 + }; + default: + break; + } + } + + // temporary back compat if (result.StatusCode == HttpStatusCode.Forbidden) { throw new AccessDeniedException($"{result.Error} Runner version v{runnerVersion} is deprecated and cannot receive messages.") @@ -120,7 +134,7 @@ namespace GitHub.Actions.RunService.WebApi }; } - throw new Exception($"Failed to get job message: {result.Error}"); + throw new Exception($"Failed to get job message. Request to {requestUri} failed with status: {result.StatusCode}. Error message {result.Error}"); } public async Task CreateSessionAsync( @@ -172,5 +186,26 @@ namespace GitHub.Actions.RunService.WebApi throw new Exception($"Failed to delete broker session: {result.Error}"); } + + private static bool TryParseErrorBody(string errorBody, out BrokerError error) + { + if (!string.IsNullOrEmpty(errorBody)) + { + try + { + error = JsonUtility.FromString(errorBody); + if (error?.Source == "actions-broker-listener") + { + return true; + } + } + catch (Exception) + { + } + } + + error = null; + return false; + } } }