NSError conformance to Debuggable breaks type casting

[terwanerik]

The latest 3.6.0 release breaks type casting, xcode mentions that a forced cast from Error to Debuggable always succeeds. I use this quite a bit in my unit tests, but i also noticed that the Logger also notices this, saying that the case is already handled, but i don't think it actually is.

Happening on line 29

public func report(error e: Error,...
switch e {
    ...
    default:
        let reason: String
        switch e {
            case let localized as LocalizedError: reason = localized.localizedDescription
            // Case is already handled by previous patterns; consider removing it
            case let convertible as CustomStringConvertible: reason = convertible.description
            default: reason = "\(e)"
        }

Looks like conforming NSError to Debuggable causes incorrect type casting. A quick test;

enum DebuggableTest: Debuggable {
    case foo

    var identifier: String {
        switch self {
        case .foo: return "foo"
        }
    }

    var reason: String {
        switch self {
        case .foo: return "FooBar"
        }
    }

    static func test() throws {
        throw DebuggableTest.foo
    }
}

func testDebuggableTest() throws {
    // Works fine, no type casting happening
    XCTAssertEqual(DebuggableTest.foo.identifier, "foo")

    XCTAssertThrowsError(try DebuggableTest.test(), "test") { err in
        // err is Error
        // Notices that a force downcast from `Error` to `Debuggable` always succeeds, so Error probably conforms to NSError?
        // Test fails, because the identifier is "0", should be "foo"
        XCTAssertEqual(DebuggableTest.foo.identifier, (err as! Debuggable).identifier)
    }

    let downcast: Error = DebuggableTest.foo
    let upcast = downcast as Debuggable

    // Prints "0" as per NSError extension, instead of "foo"
    // All of this works fine without the NSError extension
    print(upcast.identifier)
}

Is there somewhere where Error actually conforms to NSError? Since Xcode/Swift notices that the downcast always succeeds.

It doesn't actually creates a DebuggableTest but just uses the identifier getter from the NSError extension.

Is there another way to cast i should use?

[terwanerik]

NSError actually does conform to Error according to the swift docs. But that doesn't mean that every Error is a NSError, basically that's what the compiler is complaining about. But yeah the type cast should work then, right?

Since identifier and reason are in the protocol, not a protocol extension, it should use the implemented methods by the custom errors, not those of NSError, right?

[tanner0101]

That's very strange. It seems almost like let downcast: Error = DebuggableTest.foo is converting the error to an NSError. I wonder if this happens on Linux.

Maybe instead of conforming NSError to debuggable, we need to add special NSError cases anywhere we print out errors (ErrorMiddleware, Log error, etc). @MrLotU any ideas?

Also, which Swift version are you on @terwanerik? I wonder if this happens with latest Swift 5.

[terwanerik]

Yeah really strange indeed.. Also happening on Linux/Docker, found the problem because the CI stopped working on Docker (clean install every time).

Running on Swift 4.2.1!

[MrLotU]

@tanner0101 For now, I think we should revert the NSError conformance to Debuggable, since it's breaking and get a "better" solution in place for Vapor 4, and than we can really think about how we want to do that (adding extra cases or smth)