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

Display query errors from ElasticSearch #16940

Draft
wants to merge 14 commits into
base: main
Choose a base branch
from
Draft
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 @@ -3,12 +3,14 @@
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc;
using OrchardCore.Infrastructure.Filters;

namespace OrchardCore.Queries.Controllers;

[Route("api/queries")]
[ApiController]
[Authorize(AuthenticationSchemes = "Api")]
[TypeFilter(typeof(ApiExceptionHandlingFilter))]
[IgnoreAntiforgeryToken]
[AllowAnonymous]
public sealed class QueryApiController : ControllerBase
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
<ProjectReference Include="..\..\OrchardCore\OrchardCore.Data.Abstractions\OrchardCore.Data.Abstractions.csproj" />
<ProjectReference Include="..\..\OrchardCore\OrchardCore.Deployment.Abstractions\OrchardCore.Deployment.Abstractions.csproj" />
<ProjectReference Include="..\..\OrchardCore\OrchardCore.DisplayManagement\OrchardCore.DisplayManagement.csproj" />
<ProjectReference Include="..\..\OrchardCore\OrchardCore.Infrastructure\OrchardCore.Infrastructure.csproj" />
<ProjectReference Include="..\..\OrchardCore\OrchardCore.Localization.Abstractions\OrchardCore.Localization.Abstractions.csproj" />
<ProjectReference Include="..\..\OrchardCore\OrchardCore.Liquid.Abstractions\OrchardCore.Liquid.Abstractions.csproj" />
<ProjectReference Include="..\..\OrchardCore\OrchardCore.Module.Targets\OrchardCore.Module.Targets.csproj" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,33 +14,41 @@ public static Task<IEnumerable<ContentItem>> ContentQueryAsync(this IOrchardHelp

public static async Task<IEnumerable<ContentItem>> ContentQueryAsync(this IOrchardHelper orchardHelper, string queryName, IDictionary<string, object> parameters)
{
var results = await orchardHelper.QueryAsync(queryName, parameters);
var contentItems = new List<ContentItem>();

if (results != null)
var contentItems = new List<ContentItem>();
try
{
foreach (var result in results)
var results = await orchardHelper.QueryAsync(queryName, parameters);
if (results != null)
{
if (result is not ContentItem contentItem)
foreach (var result in results)
{
contentItem = null;
if (result is not ContentItem contentItem)
{
contentItem = null;

if (result is JsonObject jObject)
if (result is JsonObject jObject)
{
contentItem = jObject.ToObject<ContentItem>();
}
}

// If input is a 'JObject' but which not represents a 'ContentItem',
// a 'ContentItem' is still created but with some null properties.
if (contentItem?.ContentItemId == null)
{
contentItem = jObject.ToObject<ContentItem>();
continue;
}
}

// If input is a 'JObject' but which not represents a 'ContentItem',
// a 'ContentItem' is still created but with some null properties.
if (contentItem?.ContentItemId == null)
{
continue;
contentItems.Add(contentItem);
}

contentItems.Add(contentItem);
}
}
catch (Exception)
{
return contentItems;
}


return contentItems;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Mvc;
using OrchardCore.Entities;
using OrchardCore.Infrastructure.Filters;
using OrchardCore.Queries;
using OrchardCore.Search.Elasticsearch.Core.Services;
using OrchardCore.Search.Elasticsearch.Models;
Expand All @@ -11,6 +12,7 @@ namespace OrchardCore.Search.Elasticsearch;

[Route("api/elasticsearch")]
[ApiController]
[TypeFilter(typeof(ApiExceptionHandlingFilter))]
[Authorize(AuthenticationSchemes = "Api"), IgnoreAntiforgeryToken, AllowAnonymous]
public sealed class ElasticsearchApiController : ControllerBase
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

<ItemGroup>
<ProjectReference Include="..\..\OrchardCore\OrchardCore.Admin.Abstractions\OrchardCore.Admin.Abstractions.csproj" />
<ProjectReference Include="..\..\OrchardCore\OrchardCore.Infrastructure\OrchardCore.Infrastructure.csproj" />
<ProjectReference Include="..\..\OrchardCore\OrchardCore.Module.Targets\OrchardCore.Module.Targets.csproj" />
<ProjectReference Include="..\..\OrchardCore\OrchardCore.ContentManagement.GraphQL\OrchardCore.ContentManagement.GraphQL.csproj" />
<ProjectReference Include="..\..\OrchardCore\OrchardCore.DisplayManagement\OrchardCore.DisplayManagement.csproj" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using Microsoft.AspNetCore.Authorization;
using Microsoft.AspNetCore.Mvc;
using OrchardCore.Entities;
using OrchardCore.Infrastructure.Filters;
using OrchardCore.Queries;
using OrchardCore.Search.Lucene.Model;

Expand All @@ -10,6 +11,7 @@ namespace OrchardCore.Search.Lucene.Controllers;
[Route("api/lucene")]
[ApiController]
[Authorize(AuthenticationSchemes = "Api")]
[TypeFilter(typeof(ApiExceptionHandlingFilter))]
[IgnoreAntiforgeryToken]
[AllowAnonymous]
public sealed class LuceneApiController : ControllerBase
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
<ProjectReference Include="..\..\OrchardCore\OrchardCore.Deployment.Abstractions\OrchardCore.Deployment.Abstractions.csproj" />
<ProjectReference Include="..\..\OrchardCore\OrchardCore.DisplayManagement\OrchardCore.DisplayManagement.csproj" />
<ProjectReference Include="..\..\OrchardCore\OrchardCore.Indexing.Abstractions\OrchardCore.Indexing.Abstractions.csproj" />
<ProjectReference Include="..\..\OrchardCore\OrchardCore.Infrastructure\OrchardCore.Infrastructure.csproj" />
<ProjectReference Include="..\..\OrchardCore\OrchardCore.Localization.Abstractions\OrchardCore.Localization.Abstractions.csproj" />
<ProjectReference Include="..\..\OrchardCore\OrchardCore.Liquid.Abstractions\OrchardCore.Liquid.Abstractions.csproj" />
<ProjectReference Include="..\..\OrchardCore\OrchardCore.Module.Targets\OrchardCore.Module.Targets.csproj" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Http;
using Microsoft.AspNetCore.Mvc.Filters;
using System.Text.Json;
using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Http.Json;
using Json.More;
namespace OrchardCore.Infrastructure.Filters;

public class ApiExceptionHandlingFilter : IExceptionFilter
{
private static readonly JsonSerializerOptions _jsonOptions = new()
{
PropertyNamingPolicy = JsonNamingPolicy.CamelCase,
WriteIndented = true
};

public ApiExceptionHandlingFilter()
{

}

public void OnException(ExceptionContext context)
{
var response = new ProblemDetails
{
Type = "https://tools.ietf.org/html/rfc7231#section-6.6.1",
Title = "An error occurred while processing your request.",
Status = StatusCodes.Status500InternalServerError,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I would use a 400 for this one.

Copy link
Contributor

Choose a reason for hiding this comment

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

We could also pass an error code along with the error string that we could document. Just to make a distinction between 500 and 400.

Detail = context.Exception.Message,
Instance = context.HttpContext.Request.Path
};

context.HttpContext.Response.StatusCode = response.Status.Value;
context.Result = new JsonResult(response, _jsonOptions);
context.ExceptionHandled = true;
}
}

Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Text;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Logging;
using Nest;

Expand Down Expand Up @@ -81,6 +82,7 @@ public async Task<ElasticTopDocs> SearchAsync(string indexName, string query)
catch (Exception ex)
{
_logger.LogError(ex, "Error while querying elastic with exception: {Message}", ex.Message);
throw;
}

return elasticTopDocs;
Expand Down