Skip to content

Commit 5c18f9c

Browse files
mattpolzinmattpolzin
authored
Merge pull request #62 from mattpolzin/fix-nested-either-errors
Fix error messages for nested either decoding errors on Paths.
2 parents 56f3383 + b1280d4 commit 5c18f9c

File tree

7 files changed

+201
-80
lines changed

7 files changed

+201
-80
lines changed

‎Sources/OpenAPIKit/Encoding and Decoding Errors/DiggingError.swift‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
//
2+
// DiggingError.swift
3+
//
4+
//
5+
// Created by Mathew Polzin on 5/2/20.
6+
//
7+
8+
/// A `DiggingError` is an error that knows how to dig deeper into its
9+
/// underlying context to attempt to pull out a more granular reason for the
10+
/// failure.
11+
///
12+
/// This is a relevant concept with respect to `DecodingError`s in particular
13+
/// because they often have underlying causes.
14+
///
15+
internal protocol DiggingError {
16+
/// Initialize this error with a `DecodingError` and
17+
/// unwrap it if possible to instead a expose a more
18+
/// granular underlying error.
19+
init(unwrapping error: Swift.DecodingError)
20+
}
21+
22+
extension DiggingError {
23+
/// Returns one of the branches of the `EitherDecodeNoTypesMatchedError`
24+
/// or `nil`, depending on whether it is heuristically beneficial to dig into either
25+
/// branch in search of a more granular error.
26+
///
27+
/// If an `Either` fails to decode and the most useful error for the user is just
28+
/// that neither option of the `Either` was found, then this function returns `nil`
29+
/// which indicates neither branch should be dug into.
30+
///
31+
/// On the other hand, one of the two branches of the `Either` might have failed to decode
32+
/// trivially (i.e. the user does not need to know that both branches failed because one
33+
/// of those branches is very unlikely to have been the intended one). If that happens, it is
34+
/// more useful to dig into the less trivial branch and display a more granular error to the user
35+
/// from deeper in that brach. When this occurs, this function retruns the udnerlying error on
36+
/// that branch.
37+
internal static func eitherBranchToDigInto(_ eitherError: EitherDecodeNoTypesMatchedError) -> DecodingError? {
38+
// Just a guard against this being an error with more than 2 branches.
39+
guard eitherError.individualTypeFailures.count == 2 else { return nil }
40+
41+
let firstFailureIsReference = eitherError.individualTypeFailures[0].typeString == "$ref"
42+
let secondFailureIsReference = eitherError.individualTypeFailures[1].typeString == "$ref"
43+
44+
let firstFailureIsDeeper = eitherError.individualTypeFailures[0].codingPath(relativeTo: eitherError.codingPath).count > 1
45+
let secondFailureIsDeeper = eitherError.individualTypeFailures[1].codingPath(relativeTo: eitherError.codingPath).count > 1
46+
47+
let firstFailureNestsAnEitherError = eitherError.individualTypeFailures[0].error.underlyingError is EitherDecodeNoTypesMatchedError
48+
let secondFailureNestsAnEitherError = eitherError.individualTypeFailures[1].error.underlyingError is EitherDecodeNoTypesMatchedError
49+
50+
// The goal here is to report errors that are more granular when appropriate.
51+
// it is heuristically nice to report more granular (i.e. more deeply nested errors)
52+
// any time one of the two legs of an Either decoding failure is deeper than 1.
53+
// It is also good to dig deeper if you hit another Either decoding error immediately.
54+
55+
if firstFailureIsReference && (secondFailureIsDeeper || secondFailureNestsAnEitherError) {
56+
return eitherError.individualTypeFailures[1].error
57+
} else if secondFailureIsReference && (firstFailureIsDeeper || firstFailureNestsAnEitherError) {
58+
return eitherError.individualTypeFailures[0].error
59+
}
60+
61+
return nil
62+
}
63+
}

‎Sources/OpenAPIKit/Encoding and Decoding Errors/OperationDecodingError.swift‎

Lines changed: 17 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,23 @@ extension OpenAPI.Error.Decoding.Operation {
115115
relativeCodingPath = Array(codingPath)
116116
}
117117

118+
internal init(_ eitherError: EitherDecodeNoTypesMatchedError) {
119+
if let eitherBranchToDigInto = Self.eitherBranchToDigInto(eitherError) {
120+
self = Self(unwrapping: eitherBranchToDigInto)
121+
return
122+
}
123+
124+
var codingPath = eitherError.codingPath.dropFirst(2)
125+
// this part of the coding path is structurally guaranteed to be an HTTP verb.
126+
let verb = OpenAPI.HttpVerb(rawValue: codingPath.removeFirst().stringValue.uppercased())!
127+
128+
endpoint = verb
129+
context = .neither(eitherError)
130+
relativeCodingPath = Array(codingPath)
131+
}
132+
}
133+
134+
extension OpenAPI.Error.Decoding.Operation: DiggingError {
118135
internal init(unwrapping error: Swift.DecodingError) {
119136
if let decodingError = error.underlyingError as? Swift.DecodingError {
120137
self = Self(unwrapping: decodingError)
@@ -130,30 +147,4 @@ extension OpenAPI.Error.Decoding.Operation {
130147
self = Self(error)
131148
}
132149
}
133-
134-
internal init(_ eitherError: EitherDecodeNoTypesMatchedError) {
135-
if eitherError.individualTypeFailures.count == 2 {
136-
let firstFailureIsReference = eitherError.individualTypeFailures[0].typeString == "$ref"
137-
let secondFailureIsReference = eitherError.individualTypeFailures[1].typeString == "$ref"
138-
139-
let firstFailureIsDeeper = eitherError.individualTypeFailures[0].codingPath(relativeTo: eitherError.codingPath).count > 1
140-
let secondFailureIsDeeper = eitherError.individualTypeFailures[1].codingPath(relativeTo: eitherError.codingPath).count > 1
141-
142-
if firstFailureIsReference && secondFailureIsDeeper {
143-
self = Self(unwrapping: eitherError.individualTypeFailures[1].error)
144-
return
145-
} else if secondFailureIsReference && firstFailureIsDeeper {
146-
self = Self(unwrapping: eitherError.individualTypeFailures[0].error)
147-
return
148-
}
149-
}
150-
151-
var codingPath = eitherError.codingPath.dropFirst(2)
152-
// this part of the coding path is structurally guaranteed to be an HTTP verb.
153-
let verb = OpenAPI.HttpVerb(rawValue: codingPath.removeFirst().stringValue.uppercased())!
154-
155-
endpoint = verb
156-
context = .neither(eitherError)
157-
relativeCodingPath = Array(codingPath)
158-
}
159150
}

‎Sources/OpenAPIKit/Encoding and Decoding Errors/PathDecodingError.swift‎

Lines changed: 42 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ extension OpenAPI.Error.Decoding {
1515

1616
public enum Context {
1717
case endpoint(Operation)
18+
case inconsistency(InconsistencyError)
1819
case other(Swift.DecodingError)
1920
case neither(EitherDecodeNoTypesMatchedError)
2021
}
@@ -26,6 +27,8 @@ extension OpenAPI.Error.Decoding.Path {
2627
switch context {
2728
case .endpoint(let endpointError):
2829
return endpointError.subjectName
30+
case .inconsistency(let inconsistencyError):
31+
return inconsistencyError.subjectName
2932
case .other(let decodingError):
3033
return decodingError.subjectName
3134
case .neither(let eitherError):
@@ -52,7 +55,7 @@ extension OpenAPI.Error.Decoding.Path {
5255
case .other, .inconsistency, .neither:
5356
return "\(relativeCodingPath)for the **\(endpointError.endpoint.rawValue)** endpoint under `\(path.rawValue)`"
5457
}
55-
case .other, .neither:
58+
case .other, .neither, .inconsistency:
5659
return "\(relativeCodingPath)under the `\(path.rawValue)` path"
5760
}
5861
}
@@ -61,6 +64,8 @@ extension OpenAPI.Error.Decoding.Path {
6164
switch context {
6265
case .endpoint(let endpointError):
6366
return endpointError.errorCategory
67+
case .inconsistency(let inconsistencyError):
68+
return inconsistencyError.errorCategory
6469
case .other(let decodingError):
6570
return decodingError.errorCategory
6671
case .neither(let eitherError):
@@ -72,6 +77,8 @@ extension OpenAPI.Error.Decoding.Path {
7277
switch context {
7378
case .endpoint(let endpointError):
7479
return endpointError.codingPath
80+
case .inconsistency(let inconsistencyError):
81+
return inconsistencyError.codingPath
7582
case .other(let decodingError):
7683
return decodingError.codingPath
7784
case .neither(let eitherError):
@@ -90,7 +97,7 @@ extension OpenAPI.Error.Decoding.Path {
9097
case .other, .inconsistency, .neither:
9198
return endpointError.relativeCodingPathString
9299
}
93-
case .other, .neither:
100+
case .other, .inconsistency, .neither:
94101
return relativeCodingPath.stringValue
95102
}
96103
}
@@ -104,21 +111,49 @@ extension OpenAPI.Error.Decoding.Path {
104111
relativeCodingPath = Array(codingPath)
105112
}
106113

107-
internal init(_ eitherError: EitherDecodeNoTypesMatchedError) {
108-
var codingPath = eitherError.codingPath.dropFirst()
114+
internal init(_ error: OpenAPI.Error.Decoding.Operation) {
115+
var codingPath = error.codingPath.dropFirst()
109116
let route = OpenAPI.Path(rawValue: codingPath.removeFirst().stringValue)
110117

111118
path = route
112-
context = .neither(eitherError)
119+
context = .endpoint(error)
113120
relativeCodingPath = Array(codingPath)
114121
}
115122

116-
internal init(_ error: OpenAPI.Error.Decoding.Operation) {
123+
internal init(_ error: InconsistencyError) {
117124
var codingPath = error.codingPath.dropFirst()
118125
let route = OpenAPI.Path(rawValue: codingPath.removeFirst().stringValue)
119126

120127
path = route
121-
context = .endpoint(error)
128+
context = .inconsistency(error)
122129
relativeCodingPath = Array(codingPath)
123130
}
131+
132+
internal init(_ eitherError: EitherDecodeNoTypesMatchedError) {
133+
if let eitherBranchToDigInto = Self.eitherBranchToDigInto(eitherError) {
134+
self = Self(unwrapping: eitherBranchToDigInto)
135+
return
136+
}
137+
138+
var codingPath = eitherError.codingPath.dropFirst()
139+
let route = OpenAPI.Path(rawValue: codingPath.removeFirst().stringValue)
140+
141+
path = route
142+
context = .neither(eitherError)
143+
relativeCodingPath = Array(codingPath)
144+
}
145+
}
146+
147+
extension OpenAPI.Error.Decoding.Path: DiggingError {
148+
internal init(unwrapping error: Swift.DecodingError) {
149+
if let decodingError = error.underlyingError as? Swift.DecodingError {
150+
self = Self(unwrapping: decodingError)
151+
} else if let inconsistencyError = error.underlyingError as? InconsistencyError {
152+
self = Self(inconsistencyError)
153+
} else if let eitherError = error.underlyingError as? EitherDecodeNoTypesMatchedError {
154+
self = Self(eitherError)
155+
} else {
156+
self = Self(error)
157+
}
158+
}
124159
}

‎Sources/OpenAPIKit/Encoding and Decoding Errors/RequestDecodingError.swift‎

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,18 @@ extension OpenAPI.Error.Decoding.Request {
7777
relativeCodingPath = Self.relativePath(from: error.codingPathWithoutSubject)
7878
}
7979

80+
internal init(_ eitherError: EitherDecodeNoTypesMatchedError) {
81+
if let eitherBranchToDigInto = Self.eitherBranchToDigInto(eitherError) {
82+
self = Self(unwrapping: eitherBranchToDigInto)
83+
return
84+
}
85+
86+
context = .neither(eitherError)
87+
relativeCodingPath = Self.relativePath(from: eitherError.codingPath)
88+
}
89+
}
90+
91+
extension OpenAPI.Error.Decoding.Request: DiggingError {
8092
internal init(unwrapping error: Swift.DecodingError) {
8193
if let decodingError = error.underlyingError as? Swift.DecodingError {
8294
self = Self(unwrapping: decodingError)
@@ -88,25 +100,4 @@ extension OpenAPI.Error.Decoding.Request {
88100
self = Self(error)
89101
}
90102
}
91-
92-
internal init(_ eitherError: EitherDecodeNoTypesMatchedError) {
93-
if eitherError.individualTypeFailures.count == 2 {
94-
let firstFailureIsReference = eitherError.individualTypeFailures[0].typeString == "$ref"
95-
let secondFailureIsReference = eitherError.individualTypeFailures[1].typeString == "$ref"
96-
97-
let firstFailureIsDeeper = eitherError.individualTypeFailures[0].codingPath(relativeTo: eitherError.codingPath).count > 1
98-
let secondFailureIsDeeper = eitherError.individualTypeFailures[1].codingPath(relativeTo: eitherError.codingPath).count > 1
99-
100-
if firstFailureIsReference && secondFailureIsDeeper {
101-
self = Self(unwrapping: eitherError.individualTypeFailures[1].error)
102-
return
103-
} else if secondFailureIsReference && firstFailureIsDeeper {
104-
self = Self(unwrapping: eitherError.individualTypeFailures[0].error)
105-
return
106-
}
107-
}
108-
context = .neither(eitherError)
109-
relativeCodingPath = Self.relativePath(from: eitherError.codingPath)
110-
}
111103
}
112-

‎Sources/OpenAPIKit/Encoding and Decoding Errors/ResponseDecodingError.swift‎

Lines changed: 17 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -88,33 +88,10 @@ extension OpenAPI.Error.Decoding.Response {
8888
relativeCodingPath = Array(codingPath)
8989
}
9090

91-
internal init(unwrapping error: Swift.DecodingError) {
92-
if let decodingError = error.underlyingError as? Swift.DecodingError {
93-
self = Self(unwrapping: decodingError)
94-
} else if let inconsistencyError = error.underlyingError as? InconsistencyError {
95-
self = Self(inconsistencyError)
96-
} else if let eitherError = error.underlyingError as? EitherDecodeNoTypesMatchedError {
97-
self = Self(eitherError)
98-
} else {
99-
self = Self(error)
100-
}
101-
}
102-
10391
internal init(_ eitherError: EitherDecodeNoTypesMatchedError) {
104-
if eitherError.individualTypeFailures.count == 2 {
105-
let firstFailureIsReference = eitherError.individualTypeFailures[0].typeString == "$ref"
106-
let secondFailureIsReference = eitherError.individualTypeFailures[1].typeString == "$ref"
107-
108-
let firstFailureIsDeeper = eitherError.individualTypeFailures[0].codingPath(relativeTo: eitherError.codingPath).count > 1
109-
let secondFailureIsDeeper = eitherError.individualTypeFailures[1].codingPath(relativeTo: eitherError.codingPath).count > 1
110-
111-
if firstFailureIsReference && secondFailureIsDeeper {
112-
self = Self(unwrapping: eitherError.individualTypeFailures[1].error)
113-
return
114-
} else if secondFailureIsReference && firstFailureIsDeeper {
115-
self = Self(unwrapping: eitherError.individualTypeFailures[0].error)
116-
return
117-
}
92+
if let eitherBranchToDigInto = Self.eitherBranchToDigInto(eitherError) {
93+
self = Self(unwrapping: eitherBranchToDigInto)
94+
return
11895
}
11996

12097
var codingPath = Self.relativePath(from: eitherError.codingPath)
@@ -126,3 +103,17 @@ extension OpenAPI.Error.Decoding.Response {
126103
relativeCodingPath = Array(codingPath)
127104
}
128105
}
106+
107+
extension OpenAPI.Error.Decoding.Response: DiggingError {
108+
internal init(unwrapping error: Swift.DecodingError) {
109+
if let decodingError = error.underlyingError as? Swift.DecodingError {
110+
self = Self(unwrapping: decodingError)
111+
} else if let inconsistencyError = error.underlyingError as? InconsistencyError {
112+
self = Self(inconsistencyError)
113+
} else if let eitherError = error.underlyingError as? EitherDecodeNoTypesMatchedError {
114+
self = Self(eitherError)
115+
} else {
116+
self = Self(error)
117+
}
118+
}
119+
}

‎Sources/OpenAPIKit/Path Item/PathItem.swift‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,9 @@ extension OpenAPI.PathItem: Decodable {
276276
vendorExtensions = try Self.extensions(from: decoder)
277277
} catch let error as DecodingError {
278278

279+
throw OpenAPI.Error.Decoding.Path(error)
280+
} catch let error as InconsistencyError {
281+
279282
throw OpenAPI.Error.Decoding.Path(error)
280283
} catch let error as OpenAPI.Error.Decoding.Operation {
281284

‎Tests/OpenAPIKitErrorReportingTests/PathsErrorTests.swift‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,4 +189,51 @@ paths:
189189
)
190190
}
191191
}
192+
193+
func test_paramSchemaHasProblemDeeplyNestedInSchema() {
194+
let documentYML =
195+
"""
196+
openapi: "3.0.0"
197+
info:
198+
title: test
199+
version: 1.0
200+
paths:
201+
/hello/world:
202+
summary: hello
203+
parameters:
204+
- name: world
205+
in: query
206+
schema:
207+
type: object
208+
properties:
209+
hi:
210+
type: object
211+
items:
212+
type: string
213+
"""
214+
215+
XCTAssertThrowsError(try testDecoder.decode(OpenAPI.Document.self, from: documentYML)) { error in
216+
217+
let openAPIError = OpenAPI.Error(from: error)
218+
219+
XCTAssertEqual(
220+
openAPIError.localizedDescription,
221+
"""
222+
Inconsistency encountered when parsing `OpenAPI Schema` in .parameters[0].schema.properties.hi under the `/hello/world` path: Found schema attributes not consistent with the type specified: object.
223+
"""
224+
)
225+
XCTAssertEqual(
226+
openAPIError.codingPath.map { $0.stringValue },
227+
[
228+
"paths",
229+
"/hello/world",
230+
"parameters",
231+
"Index 0",
232+
"schema",
233+
"properties",
234+
"hi"
235+
]
236+
)
237+
}
238+
}
192239
}

0 commit comments

Comments
 (0)