-
Notifications
You must be signed in to change notification settings - Fork 41
feat: add litestar framework #311
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@KShivendu can help with the review for this. Thanks |
@Goldziher @rishabhpoddar Heres the log from console
And heres the code /im running with Uvicorn)
|
Ahh, i see now - think i might be calling the get_all_cors_headers() to early. |
Not only that, you also seem to be calling the |
Yeah, i tried that do and Get 404. See the /dashboard request also responds with 404 in the log |
It seems like when the
And this log is not present before the |
Hmm, i have no idea either. I can't actually run the tests for this, so i can't properly debug it. |
@Goldziher what error(s) are you facing when setting up the test env? We have a contributing guide. Just a note, you will need to setup the supertokens-core on your machine to run the tests (instrs on that are here: https://github.com/supertokens/supertokens-core/blob/master/CONTRIBUTING.md#development-setup) |
I have done that. I wrote in the description of the PR what im getting. |
Ah ok. I had missed that. Are you connecting it to a real database? I would suggest just setting up the core with the in memory db. The
|
Aight, i currently got the flu or something. When i feel up to it I'll give it a go and update with the results. Feel free to leave review comments regarding the code - I'll address. |
@Goldziher @rishabhpoddar
|
Ha yes, we should be using the starlite async test client |
Ive been trying to get the tests to work, but i cant! Seems like my knowledge is at its limits. Ive tried replacing all TestClient with AsyncTestClient[Litestar] without any luck.
|
Try
The test client is an async context manager |
Ok, thanks - but cant get it working. Sorry if im beeing a noob ^^
|
Try this @fixture(scope="function")
def litestar_test_client() -> TestClient[Litestar]:
return TestClient(app)
def test_login_refresh(litestar_test_client: TestClient[Litestar]):
with litestar_test_client as client:
response_1 = client.get("/login")
cookies_1 = extract_all_cookies(response_1 |
Failed, see error output.
With mode=wsgi
|
I updated the tests, its still failing: FAILED [ 5%]/Users/naamanhirschfeld/workspace/supertokens-python/supertokens_python/utils.py:200: RuntimeWarning: Inconsistent mode detected, check if you are using the right asgi / wsgi mode
warnings.warn(
tests/litestar/test_litestar.py:159 (test_login_refresh)
litestar_test_client = <litestar.testing.client.sync_client.TestClient object at 0x104881550>
def test_login_refresh(litestar_test_client: TestClient[Litestar]):
init(
supertokens_config=SupertokensConfig("http://localhost:3567"),
app_info=InputAppInfo(
app_name="SuperTokens Demo",
api_domain="http://api.supertokens.io",
website_domain="http://supertokens.io",
api_base_path="/auth",
),
framework="litestar",
recipe_list=[
session.init(
anti_csrf="VIA_TOKEN",
cookie_domain="supertokens.io",
get_token_transfer_method=get_token_transfer_method,
override=session.InputOverrideConfig(apis=apis_override_session),
)
],
mode="asgi",
)
> start_st()
test_litestar.py:180:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
host = 'localhost', port = '3567'
def start_st(host: str = "localhost", port: str = "3567"):
pid_after = pid_before = __get_list_of_process_ids()
run(
"cd "
+ INSTALLATION_PATH
+ " && java -Djava.security.egd=file:/dev/urandom -classpath "
'"./core/*:./plugin-interface/*" io.supertokens.Main ./ DEV host='
+ host
+ " port="
+ str(port)
+ " test_mode &",
shell=True,
stdout=DEVNULL,
)
for _ in range(35):
pid_after = __get_list_of_process_ids()
if len(pid_after) != len(pid_before):
break
sleep(0.5)
if len(pid_after) == len(pid_before):
> raise Exception("could not start ST process")
E Exception: could not start ST process
../utils.py:151: Exception Im afraid I dont have more time to dedicate to this. You guys are welcome to either take over this PR or close it. |
Ive also tried testing with the updated PR, without any luck - but im not getting the same error as @Goldziher . Im still getting event loop errors and i want to try and help with this but i cant figure out where to start looking for this error. It has something to do with the litestar_test_client but not sure what im looking for.
|
I dont know if @Goldziher or @rishabhpoddar are able to point me in the right direction? Im trying google and some of the answers say;
But im not sure if the answer applies here and how to fix it? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for your contribution. We really appreciate it!
I tried setting up Litestar with Supertokens Python SDK and discovered some points for improvement.
Let's resolve the error handling comment first and then we can proceed with further reviews :)
@@ -26,7 +27,7 @@ | |||
|
|||
def init( | |||
app_info: InputAppInfo, | |||
framework: Literal["fastapi", "flask", "django"], | |||
framework: SupportedFrameworks, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. Thanks!
try: | ||
result = await st.middleware( | ||
LitestarRequest(request), | ||
LitestarResponse(Response[Any](content=None)), | ||
) | ||
except SuperTokensError as e: | ||
result = await st.handle_supertokens_error( | ||
LitestarRequest(request), | ||
e, | ||
LitestarResponse(Response[Any](content=None)), | ||
) | ||
|
||
if isinstance(result, LitestarResponse): | ||
if ( | ||
session_container := request.state.get("supertokens") | ||
) and isinstance(session_container, SessionContainer): | ||
manage_session_post_response(session_container, result) | ||
|
||
await result.response(scope, receive, send) | ||
return | ||
|
||
await self.app(scope, receive, send) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
except SuperTokensError as e
isn't working as expected.
I visited http://localhost:8000/test/secure without any session (based on config given by @Spectryx) returned:
{
"status_code": 500,
"detail": "UnauthorisedError('Session does not exist. Are you sending the session tokens in the request with the appropriate token transfer method?')"
}
I used the debugger and found that await self.app(...)
raises UnauthorizedError
from verify_session
and that isn't handled here. So I changed it to:
class Middleware(AbstractMiddleware):
async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None:
st = Supertokens.get_instance()
request = Request[Any, Any, Any](scope, receive, send)
try:
result = await st.middleware(
LitestarRequest(request),
LitestarResponse(Response[Any](content=None)),
)
if isinstance(result, LitestarResponse):
if (
session_container := request.state.get("supertokens")
) and isinstance(session_container, SessionContainer):
manage_session_post_response(session_container, result)
await result.response(scope, receive, send)
return
async def send_wrapper(message: "Message") -> None:
await send(message)
await self.app(scope, receive, send_wrapper)
print("Sent response") # reaches this line
except SuperTokensError as e:
result = await st.handle_supertokens_error(
LitestarRequest(request),
e,
LitestarResponse(Response[Any](content=None)),
)
if isinstance(result, LitestarResponse):
await result.response(scope, receive, send)
return
except Exception as e:
print(e)
raise e
print("Middleware ran") # reaches this line
return Middleware
The debugger reaches print("Sent response")
line despite the error response. This seems different from other frameworks. How does Litestar handle errors? Is https://docs.litestar.dev/2/usage/exceptions.html the only way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We wrap the chain of middlewares inside an exception handling Middleware. You can customize its behavior by defining exception handler functions mapped to either status codes or exception types.
It's similar to Starlette in this regard.
Summary of change
(A few sentences about this PR)
Related issues
Test Plan
I added unit tests based on the existing ones
Documentation changes
I'm leaving this outside.
Checklist for important updates
coreDriverInterfaceSupported.json
file has been updated (if needed)supertokens_python/constants.py
frontendDriverInterfaceSupported.json
file has been updated (if needed)setup.py
supertokens_python/constants.py
git tag
) in the formatvX.Y.Z
, and then find the latest branch (git branch --all
) whoseX.Y
is greater than the latest released tag.supertokens_python/utils.py
file to include that in theFRAMEWORKS
variableRemaining TODOs for this PR
What caused the crash: java.sql.SQLException: Error opening connection
, I do not know how to fix this since its coming from the root repository (i am running it correctly in a second terminal)