Skip to content

Commit 317f293

Browse files
author
Bart Koelman
committed
Refactorings:
- Changed deserializer to accept Error(s) instead of Document (used to be ErrorDocument, but they were merged) - Write request body in error meta instead of top-level meta - Fixed: Log at Error instead of Info when processing an operation throws unknown error
1 parent 169acc7 commit 317f293

File tree

46 files changed

+575
-580
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

46 files changed

+575
-580
lines changed

‎src/JsonApiDotNetCore/AtomicOperations/OperationsProcessor.cs

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -89,15 +89,7 @@ public virtual async Task<IList<OperationContainer>> ProcessAsync(IList<Operatio
8989
catch (Exception exception)
9090
#pragma warning restore AV1210 // Catch a specific exception instead of Exception, SystemException or ApplicationException
9191
{
92-
throw new JsonApiException(new ErrorObject(HttpStatusCode.InternalServerError)
93-
{
94-
Title = "An unhandled error occurred while processing an operation in this request.",
95-
Detail = exception.Message,
96-
Source = new ErrorSource
97-
{
98-
Pointer = $"/atomic:operations[{results.Count}]"
99-
}
100-
}, exception);
92+
throw new FailedOperationException(results.Count, exception);
10193
}
10294

10395
return results;

‎src/JsonApiDotNetCore/Controllers/CoreJsonApiController.cs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,21 +14,21 @@ protected IActionResult Error(ErrorObject error)
1414
{
1515
ArgumentGuard.NotNull(error, nameof(error));
1616

17-
return Error(error.AsEnumerable());
17+
return new ObjectResult(error)
18+
{
19+
StatusCode = (int)error.StatusCode
20+
};
1821
}
1922

2023
protected IActionResult Error(IEnumerable<ErrorObject> errors)
2124
{
2225
ArgumentGuard.NotNull(errors, nameof(errors));
2326

24-
var document = new Document
25-
{
26-
Errors = errors.ToList()
27-
};
27+
ErrorObject[] errorArray = errors.ToArray();
2828

29-
return new ObjectResult(document)
29+
return new ObjectResult(errorArray)
3030
{
31-
StatusCode = (int)document.GetErrorStatusCode()
31+
StatusCode = (int)ErrorObject.GetResponseStatusCode(errorArray)
3232
};
3333
}
3434
}

‎src/JsonApiDotNetCore/Errors/FailedOperationException.cs

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
using System;
2+
using System.Net;
3+
using JetBrains.Annotations;
4+
using JsonApiDotNetCore.Serialization.Objects;
5+
6+
namespace JsonApiDotNetCore.Errors
7+
{
8+
/// <summary>
9+
/// The error that is thrown when an operation in an atomic:operations request failed to be processed for unknown reasons.
10+
/// </summary>
11+
[PublicAPI]
12+
public sealed class FailedOperationException : JsonApiException
13+
{
14+
public FailedOperationException(int operationIndex, Exception innerException)
15+
: base(new ErrorObject(HttpStatusCode.InternalServerError)
16+
{
17+
Title = "An unhandled error occurred while processing an operation in this request.",
18+
Detail = innerException.Message,
19+
Source = new ErrorSource
20+
{
21+
Pointer = $"/atomic:operations[{operationIndex}]"
22+
}
23+
}, innerException)
24+
{
25+
}
26+
}
27+
}

‎src/JsonApiDotNetCore/Errors/InvalidRequestBodyException.cs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
using System.Collections.Generic;
23
using System.Net;
34
using JetBrains.Annotations;
45
using JsonApiDotNetCore.Serialization.Objects;
@@ -11,8 +12,6 @@ namespace JsonApiDotNetCore.Errors
1112
[PublicAPI]
1213
public sealed class InvalidRequestBodyException : JsonApiException
1314
{
14-
public string RequestBody { get; }
15-
1615
public InvalidRequestBodyException(string requestBody, string genericMessage, string specificMessage, string sourcePointer,
1716
HttpStatusCode? alternativeStatusCode = null, Exception innerException = null)
1817
: base(new ErrorObject(alternativeStatusCode ?? HttpStatusCode.UnprocessableEntity)
@@ -24,10 +23,15 @@ public InvalidRequestBodyException(string requestBody, string genericMessage, st
2423
: new ErrorSource
2524
{
2625
Pointer = sourcePointer
26+
},
27+
Meta = string.IsNullOrEmpty(requestBody)
28+
? null
29+
: new Dictionary<string, object>
30+
{
31+
["RequestBody"] = requestBody
2732
}
2833
}, innerException)
2934
{
30-
RequestBody = requestBody;
3135
}
3236
}
3337
}

‎src/JsonApiDotNetCore/Middleware/AsyncJsonApiExceptionFilter.cs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System.Collections.Generic;
12
using System.Threading.Tasks;
23
using JetBrains.Annotations;
34
using JsonApiDotNetCore.Serialization.Objects;
@@ -26,11 +27,11 @@ public Task OnExceptionAsync(ExceptionContext context)
2627

2728
if (context.HttpContext.IsJsonApiRequest())
2829
{
29-
Document document = _exceptionHandler.HandleException(context.Exception);
30+
IReadOnlyList<ErrorObject> errors = _exceptionHandler.HandleException(context.Exception);
3031

31-
context.Result = new ObjectResult(document)
32+
context.Result = new ObjectResult(errors)
3233
{
33-
StatusCode = (int)document.GetErrorStatusCode()
34+
StatusCode = (int)ErrorObject.GetResponseStatusCode(errors)
3435
};
3536
}
3637

‎src/JsonApiDotNetCore/Middleware/ExceptionHandler.cs

Lines changed: 7 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,15 @@ public ExceptionHandler(ILoggerFactory loggerFactory, IJsonApiOptions options)
2727
_logger = loggerFactory.CreateLogger<ExceptionHandler>();
2828
}
2929

30-
public Document HandleException(Exception exception)
30+
public IReadOnlyList<ErrorObject> HandleException(Exception exception)
3131
{
3232
ArgumentGuard.NotNull(exception, nameof(exception));
3333

3434
Exception demystified = exception.Demystify();
3535

3636
LogException(demystified);
3737

38-
return CreateErrorDocument(demystified);
38+
return CreateErrorResponse(demystified);
3939
}
4040

4141
private void LogException(Exception exception)
@@ -55,7 +55,7 @@ protected virtual LogLevel GetLogLevel(Exception exception)
5555
return LogLevel.None;
5656
}
5757

58-
if (exception is JsonApiException)
58+
if (exception is JsonApiException and not FailedOperationException)
5959
{
6060
return LogLevel.Information;
6161
}
@@ -70,7 +70,7 @@ protected virtual string GetLogMessage(Exception exception)
7070
return exception is JsonApiException jsonApiException ? jsonApiException.GetSummary() : exception.Message;
7171
}
7272

73-
protected virtual Document CreateErrorDocument(Exception exception)
73+
protected virtual IReadOnlyList<ErrorObject> CreateErrorResponse(Exception exception)
7474
{
7575
ArgumentGuard.NotNull(exception, nameof(exception));
7676

@@ -84,25 +84,15 @@ protected virtual Document CreateErrorDocument(Exception exception)
8484
Detail = exception.Message
8585
}.AsArray();
8686

87-
var document = new Document
88-
{
89-
Errors = errors.ToList()
90-
};
91-
9287
if (_options.IncludeExceptionStackTraceInErrors && exception is not InvalidModelStateException)
9388
{
94-
IncludeStackTraces(exception, document.Errors);
95-
}
96-
97-
if (_options.IncludeRequestBodyInErrors && exception is InvalidRequestBodyException { RequestBody: { } } invalidRequestBodyException)
98-
{
99-
IncludeRequestBody(invalidRequestBodyException, document);
89+
IncludeStackTraces(exception, errors);
10090
}
10191

102-
return document;
92+
return errors;
10393
}
10494

105-
private void IncludeStackTraces(Exception exception, IList<ErrorObject> errors)
95+
private void IncludeStackTraces(Exception exception, IReadOnlyList<ErrorObject> errors)
10696
{
10797
string[] stackTraceLines = exception.ToString().Split(Environment.NewLine);
10898

@@ -115,11 +105,5 @@ private void IncludeStackTraces(Exception exception, IList<ErrorObject> errors)
115105
}
116106
}
117107
}
118-
119-
private static void IncludeRequestBody(InvalidRequestBodyException exception, Document document)
120-
{
121-
document.Meta ??= new Dictionary<string, object>();
122-
document.Meta["RequestBody"] = exception.RequestBody;
123-
}
124108
}
125109
}

‎src/JsonApiDotNetCore/Middleware/IExceptionHandler.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
using System;
2+
using System.Collections.Generic;
23
using JsonApiDotNetCore.Serialization.Objects;
34

45
namespace JsonApiDotNetCore.Middleware
@@ -8,6 +9,6 @@ namespace JsonApiDotNetCore.Middleware
89
/// </summary>
910
public interface IExceptionHandler
1011
{
11-
Document HandleException(Exception exception);
12+
IReadOnlyList<ErrorObject> HandleException(Exception exception);
1213
}
1314
}

‎src/JsonApiDotNetCore/Serialization/AtomicOperationsResponseSerializer.cs

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,13 +54,28 @@ public AtomicOperationsResponseSerializer(IResourceObjectBuilder resourceObjectB
5454
/// <inheritdoc />
5555
public string Serialize(object content)
5656
{
57-
if (content is IList<OperationContainer> operations)
57+
if (content is IEnumerable<OperationContainer> operations)
5858
{
5959
return SerializeOperationsDocument(operations);
6060
}
6161

62-
if (content is Document errorDocument)
62+
if (content is IEnumerable<ErrorObject> errors)
6363
{
64+
var errorDocument = new Document
65+
{
66+
Errors = errors.ToArray()
67+
};
68+
69+
return SerializeErrorDocument(errorDocument);
70+
}
71+
72+
if (content is ErrorObject errorObject)
73+
{
74+
var errorDocument = new Document
75+
{
76+
Errors = errorObject.AsArray()
77+
};
78+
6479
return SerializeErrorDocument(errorDocument);
6580
}
6681

‎src/JsonApiDotNetCore/Serialization/JsonApiReader.cs

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,10 +68,7 @@ private static void AssertHasRequestBody(string requestBody)
6868
{
6969
if (string.IsNullOrEmpty(requestBody))
7070
{
71-
throw new JsonApiException(new ErrorObject(HttpStatusCode.BadRequest)
72-
{
73-
Title = "Missing request body."
74-
});
71+
throw new InvalidRequestBodyException(null, "Missing request body.", null, null, HttpStatusCode.BadRequest);
7572
}
7673
}
7774

@@ -89,7 +86,7 @@ private Document DeserializeDocument(string requestBody, JsonSerializerOptions s
8986
// JsonException.Path looks great for setting error.source.pointer, but unfortunately it is wrong in most cases.
9087
// This is due to the use of custom converters, which are unable to interact with internal position tracking.
9188
// https://github.com/dotnet/runtime/issues/50205#issuecomment-808401245
92-
throw new InvalidRequestBodyException(requestBody, null, exception.Message, null, null, exception);
89+
throw new InvalidRequestBodyException(_options.IncludeRequestBodyInErrors ? requestBody : null, null, exception.Message, null, null, exception);
9390
}
9491
}
9592

@@ -101,8 +98,8 @@ private object ConvertDocumentToModel(Document document, string requestBody)
10198
}
10299
catch (ModelConversionException exception)
103100
{
104-
throw new InvalidRequestBodyException(requestBody, exception.GenericMessage, exception.SpecificMessage, exception.SourcePointer,
105-
exception.StatusCode, exception);
101+
throw new InvalidRequestBodyException(_options.IncludeRequestBodyInErrors ? requestBody : null, exception.GenericMessage,
102+
exception.SpecificMessage, exception.SourcePointer, exception.StatusCode, exception);
106103
}
107104
}
108105
}

‎src/JsonApiDotNetCore/Serialization/JsonApiWriter.cs

Lines changed: 4 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
using System;
22
using System.Collections.Generic;
33
using System.IO;
4-
using System.Linq;
54
using System.Net;
65
using System.Net.Http;
76
using System.Text;
@@ -61,10 +60,9 @@ public async Task WriteAsync(object model, HttpContext httpContext)
6160
catch (Exception exception)
6261
#pragma warning restore AV1210 // Catch a specific exception instead of Exception, SystemException or ApplicationException
6362
{
64-
Document document = _exceptionHandler.HandleException(exception);
65-
responseContent = _serializer.Serialize(document);
66-
67-
response.StatusCode = (int)document.GetErrorStatusCode();
63+
IReadOnlyList<ErrorObject> errors = _exceptionHandler.HandleException(exception);
64+
responseContent = _serializer.Serialize(errors);
65+
response.StatusCode = (int)ErrorObject.GetResponseStatusCode(errors);
6866
}
6967

7068
bool hasMatchingETag = SetETagResponseHeader(request, response, responseContent);
@@ -114,37 +112,14 @@ private string SerializeResponse(object contextObject, HttpStatusCode statusCode
114112
}
115113
}
116114

117-
object contextObjectWrapped = WrapErrors(contextObject);
118-
119-
return _serializer.Serialize(contextObjectWrapped);
115+
return _serializer.Serialize(contextObject);
120116
}
121117

122118
private bool IsSuccessStatusCode(HttpStatusCode statusCode)
123119
{
124120
return new HttpResponseMessage(statusCode).IsSuccessStatusCode;
125121
}
126122

127-
private static object WrapErrors(object contextObject)
128-
{
129-
if (contextObject is IEnumerable<ErrorObject> errors)
130-
{
131-
return new Document
132-
{
133-
Errors = errors.ToList()
134-
};
135-
}
136-
137-
if (contextObject is ErrorObject error)
138-
{
139-
return new Document
140-
{
141-
Errors = error.AsList()
142-
};
143-
}
144-
145-
return contextObject;
146-
}
147-
148123
private bool SetETagResponseHeader(HttpRequest request, HttpResponse response, string responseContent)
149124
{
150125
bool isReadOnly = request.Method == HttpMethod.Get.Method || request.Method == HttpMethod.Head.Method;

‎src/JsonApiDotNetCore/Serialization/ModelConversionException.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
11
using System;
22
using System.Net;
3+
using JetBrains.Annotations;
34
using JsonApiDotNetCore.Serialization.RequestAdapters;
45

56
namespace JsonApiDotNetCore.Serialization
67
{
78
/// <summary>
89
/// The error that is thrown when unable to convert a deserialized request body to an ASP.NET model.
910
/// </summary>
10-
internal sealed class ModelConversionException : Exception
11+
[PublicAPI]
12+
public sealed class ModelConversionException : Exception
1113
{
1214
public string GenericMessage { get; }
1315
public string SpecificMessage { get; }

‎src/JsonApiDotNetCore/Serialization/Objects/Document.cs

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,4 @@
1-
using System;
21
using System.Collections.Generic;
3-
using System.Linq;
4-
using System.Net;
52
using System.Text.Json.Serialization;
63

74
namespace JsonApiDotNetCore.Serialization.Objects
@@ -42,23 +39,5 @@ public sealed class Document
4239
[JsonPropertyName("meta")]
4340
[JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)]
4441
public IDictionary<string, object> Meta { get; set; }
45-
46-
internal HttpStatusCode GetErrorStatusCode()
47-
{
48-
if (Errors.IsNullOrEmpty())
49-
{
50-
throw new InvalidOperationException("No errors found.");
51-
}
52-
53-
int[] statusCodes = Errors.Select(error => (int)error.StatusCode).Distinct().ToArray();
54-
55-
if (statusCodes.Length == 1)
56-
{
57-
return (HttpStatusCode)statusCodes[0];
58-
}
59-
60-
int statusCode = int.Parse($"{statusCodes.Max().ToString()[0]}00");
61-
return (HttpStatusCode)statusCode;
62-
}
6342
}
6443
}

0 commit comments

Comments
 (0)