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

Simplify error handling in recipe execution #16195

Merged
merged 1 commit into from
May 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public async Task<IActionResult> Import(ImportViewModel model)
{
_logger.LogError(e, "Unable to import a recipe from deployment plan.");

await _notifier.ErrorAsync(H["The deployment plan failed with the following errors: {0}", string.Join(' ', e.StepResult.Errors.SelectMany(x => x.Value))]);
await _notifier.ErrorAsync(H["The deployment plan failed with the following errors: {0}", string.Join(' ', e.StepResult.Errors)]);
}
catch (Exception e)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
using System;
using System.IO;
using System.IO.Compression;
using System.Linq;
using System.Text.Json;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Authorization;
Expand Down Expand Up @@ -102,7 +101,7 @@ public async Task<IActionResult> Import(IFormFile importedPackage)
{
_logger.LogError(e, "Unable to import a deployment package.");

await _notifier.ErrorAsync(H["The import failed with the following errors: {0}", string.Join(' ', e.StepResult.Errors.SelectMany(x => x.Value))]);
await _notifier.ErrorAsync(H["The import failed with the following errors: {0}", string.Join(' ', e.StepResult.Errors)]);
}
catch (Exception e)
{
Expand Down Expand Up @@ -171,7 +170,7 @@ public async Task<IActionResult> Json(ImportJsonViewModel model)
{
_logger.LogError(e, "Unable to import a recipe from JSON input.");

ModelState.AddModelError(nameof(model.Json), string.Join(' ', e.StepResult.Errors.SelectMany(x => x.Value)));
ModelState.AddModelError(nameof(model.Json), string.Join(' ', e.StepResult.Errors));
}
catch (Exception e)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ public async Task<ActionResult> Execute(string basePath, string fileName)
{
_logger.LogError(e, "Unable to import a recipe file.");

await _notifier.ErrorAsync(H["The recipe '{0}' failed to run do to the following errors: {1}", recipe.DisplayName, string.Join(' ', e.StepResult.Errors.SelectMany(x => x.Value))]);
await _notifier.ErrorAsync(H["The recipe '{0}' failed to run do to the following errors: {1}", recipe.DisplayName, string.Join(' ', e.StepResult.Errors)]);
}
catch (Exception e)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
using System.Collections.Generic;

namespace OrchardCore.Recipes.Models;

public class RecipeStepResult
Expand All @@ -10,5 +8,5 @@ public class RecipeStepResult

public bool IsSuccessful { get; set; }

public Dictionary<string, string[]> Errors { get; set; }
public string[] Errors { get; set; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,7 @@ public async Task<string> ExecuteAsync(string executionId, RecipeDescriptor reci
if (recipeStep.Errors.Count > 0)
{
stepResult.IsSuccessful = false;
stepResult.Errors = new Dictionary<string, string[]>
{
{ recipeStep.Name, recipeStep.Errors.ToArray() }
};
stepResult.Errors = recipeStep.Errors.ToArray();
}
else
{
Expand All @@ -118,10 +115,7 @@ public async Task<string> ExecuteAsync(string executionId, RecipeDescriptor reci
catch (Exception e)
{
stepResult.IsSuccessful = false;
stepResult.Errors = new Dictionary<string, string[]>
{
{ recipeStep.Name, [S["Unexpected error occurred while executing the '{0}' step.", stepResult.StepName]] }
};
stepResult.Errors = [S["Unexpected error occurred while executing the '{0}' step.", stepResult.StepName]];

// Because we can't do some async processing the in catch or finally
// blocks, we store the exception to throw it later.
Expand Down
2 changes: 1 addition & 1 deletion src/OrchardCore/OrchardCore.Setup.Core/SetupService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,7 @@ await scope.ServiceProvider.GetService<IShellDescriptorManager>()
{
_logger.LogError(e, "Unable to import a recipe during setup.");

context.Errors.Add(string.Empty, string.Join(' ', e.StepResult.Errors.SelectMany(x => x.Value)));
context.Errors.Add(string.Empty, string.Join(' ', e.StepResult.Errors));
}
catch (Exception e)
{
Expand Down
2 changes: 1 addition & 1 deletion test/OrchardCore.Tests/Recipes/RecipeExecutorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ await context.UsingTenantScopeAsync(async scope =>
await recipeExecutor.ExecuteAsync(executionId, recipeDescriptor, new Dictionary<string, object>(), CancellationToken.None);
});

Assert.Contains("Unable to add content-part to the 'Message' content-type. The part name cannot be null or empty.", exception.StepResult.Errors["ContentDefinition"]);
Assert.Contains("Unable to add content-part to the 'Message' content-type. The part name cannot be null or empty.", exception.StepResult.Errors);
});
}

Expand Down