Correctly conform JWTError to AbortError
#162
Conversation
Sources/JWT/JWTError+Vapor.swift
Outdated
| @@ -1,6 +1,10 @@ | |||
| import Vapor | |||
|
|
|||
| extension JWTError: @retroactive AbortError { | |||
| public var reason: String { | |||
There was a problem hiding this comment.
@gwynne can you see any issues with this given JWTError already has var reason: String? defined? Tbh I'm surprised this actually worked but it seems to and I can't get the compiler to complain about ambiguous types no matter what I try
There was a problem hiding this comment.
I think it's a bad idea even if it seems to work, but I don't have an alternate suggestion for having AbortError conformance.
There was a problem hiding this comment.
Depends what is calling reason. If it thinks the instance is an AbortError, this will be called, otherwise if it thinks it's a JWTError, then the other one will be called.
There was a problem hiding this comment.
As long at the compiler doesn't see both and get confused we should be fine. The alternative is to wrap the error in our own type and conform that to AbortError - not as nice by far (but then we are only conforming types we own to a protocol rather than types we don't own to protocols we don't own)
These changes are now available in 5.1.1
Make sure
JWTErrornow conforms toAbortErrorso error messages are returned as expected (and as in v4).Fixes the token used in the tests so the tests now pass and add back the check for
expired.