brokenhandsio / vapor-oauth

OAuth2 Provider Library for Vapor
MIT License
133 stars 17 forks source link

Fix token scope test #13

Closed marius-se closed 1 year ago

marius-se commented 1 year ago

Looks like I accidentally removed an else block during the Vapor 4 migration.

Brett-Best commented 1 year ago

I’m not sure why this is crashing out in the tests, all tests pass when I run from Xcode.

On the macOS run it errored out with:

NIOPosix/ThreadPosix.swift:109: Precondition failed: Unable to create thread: 35

marius-se commented 1 year ago

Yeah I got the same results. Locally the tests passed on macOS and Linux/Docker. Which Xcode version are you using? I'm on 14.1.

Brett-Best commented 1 year ago

Maybe try triggering the actions again?

14.2 which is what is on GitHub Actions

marius-se commented 1 year ago

nope, that didn't help unfortunately

Brett-Best commented 1 year ago

I actually got the same error running the tests using Xcode 14.3 on my MBP.

NIOPosix/ThreadPosix.swift:109: Precondition failed: Unable to create thread: 35

alexandre-pod commented 1 year ago

Hi, I managed to fix the crash NIOPosix/ThreadPosix.swift:109: Precondition failed: Unable to create thread: 35 by adding some missing app.shutdown() across the tests.

Here the commit that fix the crashing test issue on Xcode 96113206d6e2b4c62103e03108ed11155558732d (from this fork), I am not sure if I need to do a pull request for this fix or if you can cherry-pick my changes.

0xTim commented 1 year ago

@alexandre-pod if you can create a PR into the fix/token_scope_test that would be great!

alexandre-pod commented 1 year ago

@alexandre-pod if you can create a PR into the fix/token_scope_test that would be great!

Its created: https://github.com/brokenhandsio/vapor-oauth/pull/15

0xTim commented 1 year ago

@marius-se looks like we have a CI failure now

alexandre-pod commented 1 year ago

I had tried running the tests from ubuntu and it worked, but it seems than when launching the test from ubuntu inside docker the tests crash with this error. I will try to have a look at it

NIOPosix/ThreadPosix.swift:109: Precondition failed: Unable to create thread: 11

alexandre-pod commented 1 year ago

After some investigation I was able to reproduce the issue, for that I needed to use Ubuntu 22.04.2 and it only occurs on x86 arch (The test were working on a ubuntu VM on Apple silicon).

4 tests are causing a crash:

The crash is a segmentation fault that seems to occur when capturing the stack trace during the creation of the default Abort error while creating OAuthUser.guardMiddleware() in OAuth2.swift.

I have found different solution to make the tests pass on Linux:

As I am not used to develop and debug swift app on Linux, I am out of ideas to try finding the real root cause of this issue. If anyone has an idea on how to properly fix those tests, let me know.

For now let me know if one of the "fix" I proposed is good enough for you to be added to the tests and finally merge this pull request.

marius-se commented 1 year ago

Okay at this point I'm also out of ideas. @alexandre-pod

and maybe the stranger one (and the better maybe ?) : add in those 4 failing tests a try await Task.sleep(nanosecond: 1)

Let's try this then? Seems like a bug, so we could easily remove this in the future

alexandre-pod commented 1 year ago

Here the PR with the fix #16 @marius-se