vapor-community / Imperial

Federated Authentication with OAuth providers
MIT License
153 stars 48 forks source link

Unit tests crash with "Fatal error: Unexpectedly found nil while unwrapping an Optional value" #101

Open evandelaney opened 5 months ago

evandelaney commented 5 months ago

Unit tests crash in XCTestCase subclasses inside of override func setUp() async throws at the point where .oAuth(from:authenticate:callback:scope:completion:) is called in production code.

The stack trace shows OAuthService.services.getter force unwrapping the global services variable, which is of type ThreadSpecificVariable<OAuthServiceContainer>.

According to the headerdocs for ThreadSpecificVariable.init(value:):

/// Initialize a new ThreadSpecificVariable with value for the calling thread. After calling this, the calling /// thread will see currentValue == value but on all other threads currentValue will be nil until changed.

I suspect this is a multithreading issue. Maybe the first instance of a test class (to run testFoo) sets up the OAuth route on Thread A, and the next instance of the test class (to run testBar) tries to set up the OAuth route on Thread B.

I was, in fact, able to confirm this with breakpoints in Xcode. Anytime setUp() jumps to a different thread, the test run will crash.

Here is an example minimal project that reproduces the issue.

0xTim commented 5 months ago

I believe this is caused by Swift Concurrency and the use of the thread safe wrapper. In normal code, since we're event loop based we don't hit that, but that's not the case for tests.

One option would be to migrate to a NIOLockedValueBox, that should be a quick fix that can be done in a small PR. The real solution is migrate the library to async/await, but that's going to take more time