Skip to content
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

Optional parameter "add_trailing_slash" #359

Closed
imcarlosguerrero opened this issue Jul 7, 2024 · 0 comments
Closed

Optional parameter "add_trailing_slash" #359

imcarlosguerrero opened this issue Jul 7, 2024 · 0 comments

Comments

@imcarlosguerrero
Copy link

Is your feature request related to a problem? Please describe.

I created a Socket.IO ASGI app and added a FastAPI app to it such as detailed here.

from api.app import app
import socketio

socketio_server = socketio.AsyncServer(
    async_mode="asgi",
    cors_allowed_origins="*",
)

socketio_app = socketio.ASGIApp(
    socketio_server=socketio_server,
    socketio_path="/api/socket.io",
    other_asgi_app=app,
)

Posteriorly I ran this server using uvicorn and tested the Socket.IO app using the AsyncClient such as detailed here.

#socketio_client.py
import socketio
import asyncio

sio = socketio.AsyncClient()

async def main():
    await sio.connect(
        "ws://localhost:8000",
        socketio_path="/api/socket.io/"
    )
    await sio.wait()

Everything seem to work well, so I proceeded to use the rewrites functionality of Next.js, which allows to use Next.js as frontend while using any other framework for backend in the same host, and rewrote all the traffic that would come to /api/* to http://localhost:8000/api/* which would be where my FastAPI and Socket.IO would listen for requests.

/** @type {import('next').NextConfig} */
const nextConfig = {
	async rewrites() {
		return [{source: "/api/:path*", destination: "http://localhost:8000/api/:path*"}];
	},
};

export default nextConfig;

Once this configuration has been applied I should be able to make a request to http://localhost:3000/api and receive the response from my FastAPI app, and indeed, that's what happens, I receive the right response. The problem comes when I try to test the /api/socket.io endpoint by using a modified version of the socket_client code I showed previously.

import socketio
import asyncio

sio = socketio.AsyncClient()

async def main():
    await sio.connect(
        "ws://localhost:3000",
        socketio_path="/api/socket.io/"
    )
    await sio.wait()

This code won't work, it returns the following error:

socketio.exceptions.ConnectionError: Unexpected status code 404 in server response

Checking the uvicorn logs I see that the connection request was made but the endpoint couldn't be found.

[0] INFO: 127.0.0.1:62536 - "GET /api/socket.io?transport=polling&EIO=4&t=1720303873.2073076 HTTP/1.1" 404 Not Found

I thought that Next.js broke the Socket.IO app in some mysterious way, so I tried testing the app using it's original port (8000), which to my surprise worked.

[0] INFO: 127.0.0.1:62564 - "GET /api/socket.io/?transport=polling&EIO=4&t=1720304056.1841798 HTTP/1.1" 200 OK

I believed that I had made something wrong, so I completely rewrote my entire app but got the same result, then I checked the requests and noticed that there's a small difference: The lack of the trailing slash at the bad request, this was generating the 404 not found error as /socket.io/ is a different route regarding /socket.io, so I tried the most straightforward idea, to force the Next.js rewrites to add the trailing slash but it seems like it's not possible, another thing I thought that could work would be to rewrite using the trailing slash at the end of my destination such as:

/** @type {import('next').NextConfig} */
const nextConfig = {
	async rewrites() {
		return [{source: "/api/:path*", destination: "http://localhost:8000/api/:path*/"}];
	},
};

export default nextConfig;

But sadly this results in the rewrite not working and being more like a redirect to https://localhost:8000/api/*/. I know this is a Next.js problem and I'll open a issue there to enable the trailing slash in the rewriting, but I needed my software to work as soon as possible so I continued looking into this library and found that: It is not possible to create an endpoint without the trailing slash as this code found at async_drivers/asgi.py and middleware.py don't allow it.

if not self.engineio_path.endswith('/'):
    self.engineio_path += '/'

I checked the official Socket.IO's documentation and found that adding the trailing slash when creating a server is an option that can be disabled, so I think it'd be good to have that option here too.

Describe the solution you'd like

I think that a good solution would be to create an optional add_trailing_slash parameter at the ASGIApp and WSGIApp init definitions, which default value can be True but can be changed to False in use cases like mine, I think that something like this would work:

def __init__(self, engineio_app, wsgi_app=None, static_files=None, engineio_path='engine.io', add_trailing_slash=True):
        self.engineio_app = engineio_app
        self.wsgi_app = wsgi_app
        self.engineio_path = engineio_path
        if not self.engineio_path.startswith('/'):
            self.engineio_path = '/' + self.engineio_path
        if add_trailing_slash and not self.engineio_path.endswith('/'):
            self.engineio_path += '/'
        self.static_files = static_files or {}

I have already applied this solution on both, python-engineio and python-socketio in my forks and they seem to work well, I'll open the respective pull requests for this to be checked for the maintainer.

Describe alternatives you've considered
I considered rewriting my backend in JavaScript as the option exists in Socket.IO but that would take more time than just modifying this library a bit.

Additional context
This changes should also be applied to python-socketio for this option to work correctly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant