Make MultipartParserError Public

[nakul-py]

Summary

This PR makes the MultipartParserError publicly accessible within the MultipartKit package.

Fixes #111

[ptoffy]

Thanks for this, however this won't roll as it is. If we make the enum public as it is it will be set in stone and we won't be able to update it without breaking the API. Could you follow this approach instead? https://github.com/vapor/jwt-kit/blob/main/Sources/JWTKit/JWTError.swift

[MahdiBM]

Thanks @nakul-py this is much better. We need some small changes though.

[MahdiBM]

Final changes hopefully, from my side at least.

[MahdiBM]

Looks good to me other than these 2 spaces. @ptoffy please take a look.

[ptoffy]

A few nits but looking good

[ptoffy]

Also @nakul-py this should go through a swift-format pass ideally

[adam-fowler]

With the error constructed like this there is no way to extract the reason associated values.

[adam-fowler]

There is still an issue here in that I cannot easily catch these errors without know the reason string eg

do {
    try multipartFormThings()
} catch let error as MultipartParserError where error == .invalidHeader("I need to know what string this is") {
    ...
}

@ptoffy Are the associated strings in the error necessary? If they are can we do something like this https://github.com/apple/swift-nio/blob/main/Sources/NIOFileSystem/FileSystemError.swift where the code and reason are separate.

[ptoffy]

I think as I said before we should follow JWTKit's example and add something like https://github.com/vapor/jwt-kit/blob/main/Sources/JWTKit/JWTError.swift#L90 so you can do

catch let error as MultipartParserError where error.errorType == .invalidHeader

and you then have access to the error.reason if you need it

[MahdiBM]

To be honest I'd prefer to just have an enum with a new case case __DO_NOT_USE_OR_ELSE__.
Then we can keep the pattern matching and all the enum goodies (other than exhaustive switches).

[adam-fowler]

To be honest I'd prefer to just have an enum with a new case case __DO_NOT_USE_OR_ELSE__. Then we can keep the pattern matching and all the enum goodies (other than exhaustive switches).

How does adding a DO_NOT_USE_OR_ELSE case help?

[adam-fowler]

Not sure why you need the Backing type. What is this giving us over just adding the following to MultipartParserError

let errorType: ErrorType
let reason: String?

[nakul-py]

I use the Backing type because it provides a single place to modify error-related data without exposing too much detail in MultipartParserError.
And If we later want to add more fields (e.g., identifier, underlyingError, or HTTPStatusCode), we can do so without cluttering the main struct.

@adam-fowler will you in the favour of remove Backing type.

[MahdiBM]

How does adding a __DO_NOT_USE_OR_ELSE__ case help?

Well if there is such a case, it solves the exhaustive switch problem. Then we can just mark the enum as public.
I saw it somewhere else before too ... an Apple repo I think ... I think even Fabian approved that? can't remember more.

[0xTim]

Well if there is such a case, it solves the exhaustive switch problem. Then we can just mark the enum as public.
I saw it somewhere else before too ... an Apple repo I think ... I think even Fabian approved that? can't remember more.

I'd vote not to use this - it's not a particularly nice solution and means that any new cases get blackholed into that catch all that just causes confusion because now people have to switch over the DO_NOT_USE_OR_ELSE case. We should stick with the approach we've taken in other repos until non-frozen enums land

[MahdiBM]

It does have downsides, but the upside is it's much easier to maintain and you can use associated values with each case in a much nicer way. Also get the pattern matching stuff like in catch blocks.

I would go this route personally, but I don't think y'all like this (I mean you mentioned it too) so I'm just trying to explain the approach.
For this PR we can go whatever sensible route you'd want, like what we have in JWTKit, PostgresNIO etc...