Exception handling: Improve error messaging on invalid umbraco-package.json file (#20332)

* Improve error messaging on invalid umbraco-package.json file.

* Adjust failing unit tests

---------

Co-authored-by: Laura Neto <12862535+lauraneto@users.noreply.github.com>
This commit is contained in:
Andy Butland
2025-10-02 13:03:00 +02:00
committed by GitHub
parent cf61356b80
commit 436be6ec3f
3 changed files with 32 additions and 43 deletions

View File

@@ -1,4 +1,4 @@
using System.Text;
using System.Text;
using Microsoft.AspNetCore.Html;
using Microsoft.AspNetCore.Mvc.Rendering;
using Umbraco.Cms.Core;
@@ -24,29 +24,18 @@ public static class HtmlHelperBackOfficeExtensions
IBackOfficePathGenerator backOfficePathGenerator,
IPackageManifestService packageManifestService)
{
try
{
PackageManifestImportmap packageImports = await packageManifestService.GetPackageManifestImportmapAsync();
PackageManifestImportmap packageImports = await packageManifestService.GetPackageManifestImportmapAsync();
var sb = new StringBuilder();
sb.AppendLine("""<script type="importmap">""");
sb.AppendLine(jsonSerializer.Serialize(packageImports));
sb.AppendLine("</script>");
var sb = new StringBuilder();
sb.AppendLine("""<script type="importmap">""");
sb.AppendLine(jsonSerializer.Serialize(packageImports));
sb.AppendLine("</script>");
// Inject the BackOffice cache buster into the import string to handle BackOffice assets
var importmapScript = sb.ToString()
.Replace(backOfficePathGenerator.BackOfficeVirtualDirectory, backOfficePathGenerator.BackOfficeAssetsPath)
.Replace(Constants.Web.CacheBusterToken, backOfficePathGenerator.BackOfficeCacheBustHash);
// Inject the BackOffice cache buster into the import string to handle BackOffice assets
var importmapScript = sb.ToString()
.Replace(backOfficePathGenerator.BackOfficeVirtualDirectory, backOfficePathGenerator.BackOfficeAssetsPath)
.Replace(Constants.Web.CacheBusterToken, backOfficePathGenerator.BackOfficeCacheBustHash);
return html.Raw(importmapScript);
}
catch (NotSupportedException ex)
{
throw new NotSupportedException("Failed to serialize the BackOffice import map", ex);
}
catch (Exception ex)
{
throw new Exception("Failed to generate the BackOffice import map", ex);
}
return html.Raw(importmapScript);
}
}

View File

@@ -1,4 +1,5 @@
using System.Text;
using System.Text;
using System.Text.Json;
using Microsoft.Extensions.FileProviders;
using Microsoft.Extensions.Logging;
using Umbraco.Cms.Core.IO;
@@ -92,10 +93,15 @@ internal class PackageManifestReader : IPackageManifestReader
packageManifests.Add(packageManifest);
}
}
catch (JsonException ex)
{
throw new InvalidOperationException(
$"The package manifest file {fileInfo.PhysicalPath} could not be parsed as it does not contain valid JSON. Please see the inner exception for details.", ex);
}
catch (Exception ex)
{
_logger.LogError(ex, "Unable to load package manifest file: {FileName}", fileInfo.Name);
throw;
throw new InvalidOperationException(
$"The package manifest file {fileInfo.PhysicalPath} could not be parsed due to an unexpected error. Please see the inner exception for details.", ex);
}
}

View File

@@ -186,8 +186,9 @@ public class PackageManifestReaderTests
.Setup(f => f.GetEnumerator())
.Returns(new List<IFileInfo> { CreatePackageManifestFile(content) }.GetEnumerator());
Assert.ThrowsAsync<JsonException>(() => _reader.ReadPackageManifestsAsync());
EnsureLogErrorWasCalled();
var exception = Assert.ThrowsAsync<InvalidOperationException>(() => _reader.ReadPackageManifestsAsync());
Assert.NotNull(exception);
Assert.IsInstanceOf<JsonException>(exception.InnerException);
}
[Test]
@@ -202,8 +203,9 @@ public class PackageManifestReaderTests
.Setup(f => f.GetEnumerator())
.Returns(new List<IFileInfo> { CreatePackageManifestFile(content) }.GetEnumerator());
Assert.ThrowsAsync<JsonException>(() => _reader.ReadPackageManifestsAsync());
EnsureLogErrorWasCalled();
var exception = Assert.ThrowsAsync<InvalidOperationException>(() => _reader.ReadPackageManifestsAsync());
Assert.NotNull(exception);
Assert.IsInstanceOf<JsonException>(exception.InnerException);
}
[Test]
@@ -224,8 +226,9 @@ public class PackageManifestReaderTests
.Setup(f => f.GetEnumerator())
.Returns(new List<IFileInfo> { CreatePackageManifestFile(content) }.GetEnumerator());
Assert.ThrowsAsync<JsonException>(() => _reader.ReadPackageManifestsAsync());
EnsureLogErrorWasCalled();
var exception = Assert.ThrowsAsync<InvalidOperationException>(() => _reader.ReadPackageManifestsAsync());
Assert.NotNull(exception);
Assert.IsInstanceOf<JsonException>(exception.InnerException);
}
[TestCase("This is not JSON")]
@@ -236,20 +239,11 @@ public class PackageManifestReaderTests
.Setup(f => f.GetEnumerator())
.Returns(new List<IFileInfo> { CreatePackageManifestFile(content) }.GetEnumerator());
Assert.ThrowsAsync<JsonException>(() => _reader.ReadPackageManifestsAsync());
EnsureLogErrorWasCalled();
var exception = Assert.ThrowsAsync<InvalidOperationException>(() => _reader.ReadPackageManifestsAsync());
Assert.NotNull(exception);
Assert.IsInstanceOf<JsonException>(exception.InnerException);
}
private void EnsureLogErrorWasCalled(int numberOfTimes = 1) =>
_loggerMock.Verify(
x => x.Log(
It.Is<LogLevel>(l => l == LogLevel.Error),
It.IsAny<EventId>(),
It.Is<It.IsAnyType>((v, t) => true),
It.IsAny<Exception>(),
It.Is<Func<It.IsAnyType, Exception, string>>((v, t) => true)),
Times.Exactly(numberOfTimes));
private IFileInfo CreateDirectoryMock(string path, params IFileInfo[] children)
{
var directoryContentsMock = new Mock<IDirectoryContents>();