Merge pull request #157 from mattpolzin/less-crashing-more-errors

Fix crash on decoding error for responses within components object.
mattpolzin · Oct 24, 2020 · f534aa9 · f534aa9
2 parents 55e8672 + 7fc5a99
commit f534aa9
Show file tree

Hide file tree

Showing 7 changed files with 60 additions and 15 deletions.
diff --git a/Sources/OpenAPIKit/Encoding and Decoding Errors/ResponseDecodingError.swift b/Sources/OpenAPIKit/Encoding and Decoding Errors/ResponseDecodingError.swift
@@ -31,7 +31,7 @@ extension OpenAPI.Error.Decoding.Response {
         }
     }
 
-    public var contextString: String { statusCode.rawValue }
+    public var contextString: String { "" }
 
     public var errorCategory: ErrorCategory {
         switch context {
@@ -70,8 +70,10 @@ extension OpenAPI.Error.Decoding.Response {
         var codingPath = Self.relativePath(from: error.codingPath)
         let code = codingPath.removeFirst().stringValue.lowercased()
 
-        // this part of the coding path is structurally guaranteed to be a status code.
-        statusCode = OpenAPI.Response.StatusCode(rawValue: code)!
+        // this part of the coding path is structurally guaranteed to be a status code
+        // unless we are in the components in which case the status code is not
+        // relevant.
+        statusCode = OpenAPI.Response.StatusCode(rawValue: code) ?? .default
         context = .inconsistency(error)
         relativeCodingPath = Array(codingPath)
     }
@@ -80,8 +82,10 @@ extension OpenAPI.Error.Decoding.Response {
         var codingPath = Self.relativePath(from: error.codingPathWithoutSubject)
         let code = codingPath.removeFirst().stringValue.lowercased()
 
-        // this part of the coding path is structurally guaranteed to be a status code.
-        statusCode = OpenAPI.Response.StatusCode(rawValue: code)!
+        // this part of the coding path is structurally guaranteed to be a status code
+        // unless we are in the components in which case the status code is not
+        // relevant.
+        statusCode = OpenAPI.Response.StatusCode(rawValue: code) ?? .default
         context = .other(error)
         relativeCodingPath = Array(codingPath)
     }
@@ -95,8 +99,10 @@ extension OpenAPI.Error.Decoding.Response {
         var codingPath = Self.relativePath(from: eitherError.codingPath)
         let code = codingPath.removeFirst().stringValue.lowercased()
 
-        // this part of the coding path is structurally guaranteed to be a status code.
-        statusCode = OpenAPI.Response.StatusCode(rawValue: code)!
+        // this part of the coding path is structurally guaranteed to be a status code
+        // unless we are in the components in which case the status code is not
+        // relevant.
+        statusCode = OpenAPI.Response.StatusCode(rawValue: code) ?? .default
         context = .neither(eitherError)
         relativeCodingPath = Array(codingPath)
     }

diff --git a/Sources/OpenAPIKit/Header/Header.swift b/Sources/OpenAPIKit/Header/Header.swift
@@ -283,10 +283,16 @@ extension OpenAPI.Header: Decodable {
             schemaOrContent = .init(content)
         case (nil, let schema?):
             schemaOrContent = .init(schema)
-        default:
+        case (nil, nil):
             throw InconsistencyError(
                 subjectName: "Header",
-                details: "A single path parameter must specify one but not both `content` and `schema`",
+                details: "A header parameter must specify either `content` or `schema`",
+                codingPath: decoder.codingPath
+            )
+        case (_, _):
+            throw InconsistencyError(
+                subjectName: "Header",
+                details: "A header must specify one but not both `content` and `schema`",
                 codingPath: decoder.codingPath
             )
         }

diff --git a/Sources/OpenAPIKit/Parameter/Parameter.swift b/Sources/OpenAPIKit/Parameter/Parameter.swift
@@ -260,10 +260,16 @@ extension OpenAPI.Parameter: Decodable {
             schemaOrContent = .init(content)
         case (nil, let schema?):
             schemaOrContent = .init(schema)
-        default:
+        case (nil, nil):
             throw InconsistencyError(
                 subjectName: name,
-                details: "A single path parameter must specify one but not both `content` and `schema`",
+                details: "A parameter must specify either `content` or `schema`",
+                codingPath: decoder.codingPath
+            )
+        case (_, _):
+            throw InconsistencyError(
+                subjectName: name,
+                details: "A parameter must specify one but not both `content` and `schema`",
                 codingPath: decoder.codingPath
             )
         }

diff --git a/Tests/OpenAPIKitCompatibilitySuite/GitHubAPITests.swift b/Tests/OpenAPIKitCompatibilitySuite/GitHubAPITests.swift
@@ -33,7 +33,7 @@ final class GitHubAPICampatibilityTests: XCTestCase {
             githubAPI = Result {
                 try YAMLDecoder().decode(
                     OpenAPI.Document.self,
-                    from: String(contentsOf: URL(string: "https://raw.githubusercontent.com/github/rest-api-description/main/descriptions/ghes-2.21/ghes-2.21.yaml")!)
+                    from: String(contentsOf: URL(string: "https://raw.githubusercontent.com/github/rest-api-description/v1.0.0-rc.1/descriptions/ghes-2.21/ghes-2.21.yaml")!)
                 )
             }
         }

diff --git a/Tests/OpenAPIKitErrorReportingTests/ComponentErrorTests.swift b/Tests/OpenAPIKitErrorReportingTests/ComponentErrorTests.swift
@@ -34,4 +34,31 @@ final class ComponentErrorTests: XCTestCase {
             XCTAssertEqual(openAPIError.codingPath.map { $0.stringValue }, ["components", "schemas", "h#llo"])
         }
     }
+
+    func test_badResponseBecauseOfHeaderInsideComponents() {
+        let documentYML =
+        """
+        openapi: "3.0.0"
+        info:
+            title: test
+            version: 1.0
+        paths: {}
+        components:
+            responses:
+                IllFormed:
+                    description: hello world
+                    headers:
+                        MissingSchemaKey:
+                            type: string # should be nested within a `schema` object
+                    content: {}
+        """
+
+        XCTAssertThrowsError(try testDecoder.decode(OpenAPI.Document.self, from: documentYML)) { error in
+
+            let openAPIError = OpenAPI.Error(from: error)
+
+            XCTAssertEqual(openAPIError.localizedDescription, "Found neither a $ref nor a Header. \n\nHeader could not be decoded because:\nInconsistency encountered when parsing `Header`: A header parameter must specify either `content` or `schema`..")
+            XCTAssertEqual(openAPIError.codingPath.map { $0.stringValue }, ["components", "responses", "IllFormed", "headers", "MissingSchemaKey"])
+        }
+    }
 }
diff --git a/Tests/OpenAPIKitErrorReportingTests/PathsErrorTests.swift b/Tests/OpenAPIKitErrorReportingTests/PathsErrorTests.swift
@@ -137,7 +137,7 @@ final class PathsErrorTests: XCTestCase {
 
             let openAPIError = OpenAPI.Error(from: error)
 
-            XCTAssertEqual(openAPIError.localizedDescription, "Found neither a $ref nor a Parameter in .parameters[0] under the `/hello/world` path. \n\nParameter could not be decoded because:\nInconsistency encountered when parsing `world`: A single path parameter must specify one but not both `content` and `schema`.."
+            XCTAssertEqual(openAPIError.localizedDescription, "Found neither a $ref nor a Parameter in .parameters[0] under the `/hello/world` path. \n\nParameter could not be decoded because:\nInconsistency encountered when parsing `world`: A parameter must specify either `content` or `schema`.."
             )
             XCTAssertEqual(
                 openAPIError.codingPath.map { $0.stringValue },
@@ -176,7 +176,7 @@ final class PathsErrorTests: XCTestCase {
 
             let openAPIError = OpenAPI.Error(from: error)
 
-            XCTAssertEqual(openAPIError.localizedDescription, "Found neither a $ref nor a Parameter in .parameters[0] under the `/hello/world` path. \n\nParameter could not be decoded because:\nInconsistency encountered when parsing `world`: A single path parameter must specify one but not both `content` and `schema`.."
+            XCTAssertEqual(openAPIError.localizedDescription, "Found neither a $ref nor a Parameter in .parameters[0] under the `/hello/world` path. \n\nParameter could not be decoded because:\nInconsistency encountered when parsing `world`: A parameter must specify one but not both `content` and `schema`.."
             )
             XCTAssertEqual(
                 openAPIError.codingPath.map { $0.stringValue },

diff --git a/Tests/OpenAPIKitErrorReportingTests/ResponseErrorTests.swift b/Tests/OpenAPIKitErrorReportingTests/ResponseErrorTests.swift
@@ -39,7 +39,7 @@ final class ResponseErrorTests: XCTestCase {
 
             let openAPIError = OpenAPI.Error(from: error)
 
-            XCTAssertEqual(openAPIError.localizedDescription, "Found neither a $ref nor a Header in .headers.hi for the status code '200' response of the **GET** endpoint under `/hello/world`. \n\nHeader could not be decoded because:\nInconsistency encountered when parsing `Header`: A single path parameter must specify one but not both `content` and `schema`..")
+            XCTAssertEqual(openAPIError.localizedDescription, "Found neither a $ref nor a Header in .headers.hi for the status code '200' response of the **GET** endpoint under `/hello/world`. \n\nHeader could not be decoded because:\nInconsistency encountered when parsing `Header`: A header must specify one but not both `content` and `schema`..")
             XCTAssertEqual(openAPIError.codingPath.map { $0.stringValue }, [
                 "paths",
                 "/hello/world",