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.

Sign up for GitHub

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

Jump to bottom

Fixes Strawberry shake exception handling (#4596) #6039

Merged
merged 2 commits into from
Apr 11, 2023
Merged

Fixes Strawberry shake exception handling (#4596) #6039

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a24bd5f
Fixes Strawberry shake exception handling (#4596)
chrisdrobison Apr 10, 2023
98d62a0
Fixed the fix
michaelstaib Apr 11, 2023
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
6 changes: 3 additions & 3 deletions src/HotChocolate/Core/src/Types/Types/Descriptors/ObjectFieldDescriptor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ public class ObjectFieldDescriptor
private ParameterInfo[] _parameterInfos = Array.Empty<ParameterInfo>();

/// <summary>
/// Creates a new instance of <see cref="ObjectFieldDescriptor"/>
/// Creates a new instance of <see cref="ObjectFieldDescriptor"/>
/// </summary>
protected ObjectFieldDescriptor(
IDescriptorContext context,
Expand All @@ -38,7 +38,7 @@ protected ObjectFieldDescriptor(
}

/// <summary>
/// Creates a new instance of <see cref="ObjectFieldDescriptor"/>
/// Creates a new instance of <see cref="ObjectFieldDescriptor"/>
/// </summary>
protected ObjectFieldDescriptor(
IDescriptorContext context,
Expand Down Expand Up @@ -76,7 +76,7 @@ protected ObjectFieldDescriptor(
}

/// <summary>
/// Creates a new instance of <see cref="ObjectFieldDescriptor"/>
/// Creates a new instance of <see cref="ObjectFieldDescriptor"/>
/// </summary>
protected ObjectFieldDescriptor(
IDescriptorContext context,
Expand Down
17 changes: 17 additions & 0 deletions src/StrawberryShake/Client/src/Core/OperationResultBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,23 @@ public IOperationResult<TResultData> Build(
errors = list;
}

// If we have a transport error but the response does not contain any client errors
// we will create a client error from the provided transport error.
if (response.Exception is not null && errors is not { Count: > 0 })
{
errors = new IClientError[]
{
new ClientError(
response.Exception.Message,
ErrorCodes.InvalidResultDataStructure,
exception: response.Exception,
extensions: new Dictionary<string, object?>
{
{ nameof(response.Exception.StackTrace), response.Exception.StackTrace }
})
};
}

return new OperationResult<TResultData>(
data,
dataInfo,
Expand Down
5 changes: 3 additions & 2 deletions src/StrawberryShake/Client/src/Core/Response.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using StrawberryShake.Properties;
using static StrawberryShake.Properties.Resources;

namespace StrawberryShake;
Expand Down Expand Up @@ -38,7 +39,7 @@ public sealed class Response<TBody> : IDisposable where TBody : class
/// Additional custom data provided by client extensions.
/// </param>
public Response(
TBody body,
TBody? body,
Exception? exception,
bool isPatch = false,
bool hasNext = false,
Expand All @@ -61,7 +62,7 @@ public Response(
/// <summary>
/// The serialized response body.
/// </summary>
public TBody Body { get; }
public TBody? Body { get; }

/// <summary>
/// The transport exception.
Expand Down
6 changes: 6 additions & 0 deletions src/StrawberryShake/Client/src/Resources/Properties/Resources.Designer.cs
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions src/StrawberryShake/Client/src/Resources/Properties/Resources.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,4 +132,7 @@
<data name="JsonResultPatcher_PathSegmentMustBeStringOrInt" xml:space="preserve">
<value>A path segment must be a string or an integer.</value>
</data>
<data name="ResponseEnumerator_HttpNoSuccessStatusCode" xml:space="preserve">
<value>Response status code does not indicate success: {0} ({1}).</value>
</data>
</root>
36 changes: 33 additions & 3 deletions src/StrawberryShake/Client/src/Transport.Http/ResponseEnumerator.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using Microsoft.AspNetCore.WebUtilities;
using static System.Net.Http.HttpCompletionOption;
using static System.StringComparison;
using static StrawberryShake.Properties.Resources;
using static StrawberryShake.Transport.Http.ResponseHelper;

namespace StrawberryShake.Transport.Http;
Expand Down Expand Up @@ -67,8 +68,37 @@ public async ValueTask<bool> MoveNextAsync()
{
try
{
response.EnsureSuccessStatusCode();
Current = await stream.TryParseResponse(_abort).ConfigureAwait(false);
Exception? transportError = null;

// If we detect that the response has a non-success status code we will
// create a transport error that will be added to the response.
if (!response.IsSuccessStatusCode)
{
#if NET5_0_OR_GREATER
transportError =
new HttpRequestException(
string.Format(
ResponseEnumerator_HttpNoSuccessStatusCode,
(int)response.StatusCode,
response.ReasonPhrase),
null,
response.StatusCode);
#else
transportError =
new HttpRequestException(
string.Format(
ResponseEnumerator_HttpNoSuccessStatusCode,
(int)response.StatusCode,
response.ReasonPhrase),
null);
#endif
}

// We now try to parse the possible GraphQL response, this step could fail
// as the response might not be a GraphQL response. It could in some cases
// be a HTML error page.
Current = await stream.TryParseResponse(transportError, _abort)
.ConfigureAwait(false);
}
catch (Exception ex)
{
Expand All @@ -93,7 +123,7 @@ public async ValueTask<bool> MoveNextAsync()
using var body = multipartSection.Body;
#endif

Current = await body.TryParseResponse(_abort).ConfigureAwait(false);
Current = await body.TryParseResponse(null, _abort).ConfigureAwait(false);

if (Current.Exception is not null)
{
Expand Down
9 changes: 6 additions & 3 deletions src/StrawberryShake/Client/src/Transport.Http/ResponseHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
using System;
using System.Buffers;
using System.IO;
using System.Net.Http;
using System.Text.Json;
using System.Threading;
using System.Threading.Tasks;
Expand All @@ -13,6 +14,7 @@ internal static class ResponseHelper
{
public static async Task<Response<JsonDocument>> TryParseResponse(
this Stream stream,
Exception? transportError,
CancellationToken cancellationToken)
{
try
Expand All @@ -35,14 +37,15 @@ await JsonDocument.ParseAsync(
hasNext = true;
}

return new Response<JsonDocument>(document, null, isPatch, hasNext);
return new Response<JsonDocument>(document, transportError, isPatch, hasNext);
}

return new Response<JsonDocument>(document, null);
return new Response<JsonDocument>(document, transportError);
}
catch (Exception ex)
{
return new Response<JsonDocument>(CreateBodyFromException(ex), ex);
var error = transportError ?? ex;
return new Response<JsonDocument>(CreateBodyFromException(error), error);
}
}

Expand Down
119 changes: 80 additions & 39 deletions src/StrawberryShake/Client/test/Transport.WebSocket.Tests/TestHelper/TestServerHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
using System;
using HotChocolate;
using HotChocolate.Execution;
using HotChocolate.Execution.Configuration;
using HotChocolate.StarWars;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using static HotChocolate.WellKnownContextData;

namespace StrawberryShake.Transport.WebSockets;

Expand All @@ -24,55 +27,93 @@ public static IWebHost CreateServer(Action<IRequestExecutorBuilder> configure, o
var host = new WebHostBuilder()
.UseConfiguration(config)
.UseKestrel()
.ConfigureServices(services =>
{
var builder = services.AddRouting().AddGraphQLServer();
.ConfigureServices(
services =>
{
var builder = services.AddRouting().AddGraphQLServer();

configure(builder);
configure(builder);

builder
.AddStarWarsTypes()
.AddExportDirectiveType()
.AddStarWarsRepositories()
.AddInMemorySubscriptions()
.ModifyOptions(
o =>
{
o.EnableDefer = true;
o.EnableStream = true;
});
})
.Configure(app =>
app.Use(async (ct, next) =>
{
try
{
// Kestrel does not return proper error responses:
// https://github.com/aspnet/KestrelHttpServer/issues/43
await next();
}
catch (Exception ex)
{
if (ct.Response.HasStarted)
builder
.AddStarWarsTypes()
.AddExportDirectiveType()
.AddStarWarsRepositories()
.AddInMemorySubscriptions()
.ModifyOptions(
o =>
{
throw;
}
o.EnableDefer = true;
o.EnableStream = true;
})
.UseDefaultPipeline()
.UseRequest(
next => async context =>
{
if (context.ContextData.TryGetValue(
nameof(HttpContext),
out var value) &&
value is HttpContext httpContext &&
context.Result is IQueryResult result)
{
var headers = httpContext.Request.Headers;
if (headers.ContainsKey("sendErrorStatusCode"))
{
context.Result = result =
QueryResultBuilder
.FromResult(result)
.SetContextData(HttpStatusCode, 403)
.Create();
}

if (headers.ContainsKey("sendError"))
{
context.Result =
QueryResultBuilder
.FromResult(result)
.AddError(new Error("Some error!"))
.Create();
}
}

await next(context);
});
})
.Configure(
app =>
app.Use(
async (ct, next) =>
{
try
{
// Kestrel does not return proper error responses:
// https://github.com/aspnet/KestrelHttpServer/issues/43
await next();
}
catch (Exception ex)
{
if (ct.Response.HasStarted)
{
throw;
}

ct.Response.StatusCode = 500;
ct.Response.Headers.Clear();
await ct.Response.WriteAsync(ex.ToString());
}
})
.UseWebSockets()
.UseRouting()
.UseEndpoints(e => e.MapGraphQL()))
ct.Response.StatusCode = 500;
ct.Response.Headers.Clear();
await ct.Response.WriteAsync(ex.ToString());
}
})
.UseWebSockets()
.UseRouting()
.UseEndpoints(e => e.MapGraphQL()))
.Build();

host.Start();

return host;
}
catch { }
catch
{
// we ignore any errors here and try the next port
}
}

throw new InvalidOperationException("Not port found");
Expand Down
Loading