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

Use a webframework instead of raw TCP #7

Open
LoipesMas opened this issue Jul 24, 2022 · 10 comments
Open

Use a webframework instead of raw TCP #7

LoipesMas opened this issue Jul 24, 2022 · 10 comments
Labels
difficulty: hard Not suited for newcomers enhancement New feature or request
Milestone

Comments

@LoipesMas
Copy link
Contributor

Looking at the server code, it seems that a big part of it (handling connections, encryption, reading from sockets) could be removed if you would use a web framework (such as rocket or actix).
This could improve code readability (less code, better structured), security/stability (replacing custom implementations with tested ones) and perhaps even performance. Also using a more standard way would make it easier to grasp what's going on and expand the functionalities.

What do you think? Or is there a reason why you chose to not use a web framework?

@Flone-dnb
Copy link
Owner

Flone-dnb commented Jul 24, 2022

Initially I started using raw sockets because I've written code in the past using raw sockets.

Eventually we should probably switch to something like actix but right now I don't see a good reason for this switch (because it would require a lot of work). Current networking code is... done. Network messaging is implemented in the 'shared' crate and right now every crate of this project just uses functions like send_message or receive_message and adding new messages is as simple as adding a new entry to an enum. I haven't encountered any critical issues with current networking so I don't see a reason for this switch. The only issue that I see right now is that we are dedicated a whole thread for each connection (which is obviously not a good thing).

I had some experience with actix for web servers in the past, should we use something like websockets or something else would be more appropriate here?

@Flone-dnb Flone-dnb added the enhancement New feature or request label Jul 24, 2022
@LoipesMas
Copy link
Contributor Author

For a (relatively) simple performance improvement we could replace std with tokio which is async.

I'd argue that it's better to rewrite it sooner than later, because you have less code to rewrite and you won't write code that will be rewritten later.
But if you don't plan on changing/extending current network code, then this can wait.

Also I was thinking about a HTTP server, especially for reporting (because there you make a single request once in a while). For clients HTTP server should work too, because you don't really have a stream of data. But websockets should be fine too.

@Flone-dnb
Copy link
Owner

I'd argue that it's better to rewrite it sooner than later, because you have less code to rewrite and you won't write code that will be rewritten later.

Agreed, but I don't want to change something that works. Moreover even right now it would require a lot of time to rewrite this (the time that I don't and probably won't have in the future).

As I've said, eventually we should totally do this. But right now we have more important stuff to implement.

So I'm putting this issue on hold, until somebody will start implementing this.

@Flone-dnb Flone-dnb added the help wanted Extra attention is needed label Jul 24, 2022
@LoipesMas
Copy link
Contributor Author

Sure, that's reasonable.

@Flone-dnb Flone-dnb added the difficulty: hard Not suited for newcomers label Aug 2, 2022
@Flone-dnb
Copy link
Owner

@LoipesMas I got interested in Salvo, do you think this can fit our needs? We can then remove the client application to just have a web interface that the server provides.

@Flone-dnb Flone-dnb added this to the v2.0.0 milestone Dec 17, 2022
@Flone-dnb Flone-dnb removed the help wanted Extra attention is needed label Feb 20, 2023
@Flone-dnb Flone-dnb removed this from the v2.0.0 milestone May 18, 2023
@Flone-dnb Flone-dnb added the question Further information is requested label May 18, 2023
@Flone-dnb
Copy link
Owner

Flone-dnb commented May 18, 2023

So I just gave this a quick though and although I know that there are some great pros for using a web framework but wouldn't it require our users to spend money on SSL certificates? I only know some base information on the web topic so help me out here.

I'm pretty sure most of our users are indie developers, who probably don't want to spend much money/time on setting up certificates, remembering to renew them and just deal with all this certificate stuff (since probably they will already spend a little money on a cloud Linux machine to host the server). Our current solution requires almost no effort to set everything up and it will be secured (relatively) enough. I mean this is not some bank management application, bug reports don't hold any sensitive information except maybe for the email addresses (if provided by the reporter).

Don't get me wrong, I would actually like to switch to some library that handles networking but only if it won't require our users to do some complicated setups or spend more money so help me out here.

P.S. Moreover, maybe #3 will be enough for most of our users since quite a lot of people were asking for it and we won't even need to care about doing all networking ourselves.

@gedw99
Copy link

gedw99 commented May 21, 2023

hey @Flone-dnb

Maybe NATS fits the bill here... Its tcp, has create rust and golang support. scales like nuts. have built in security if you want it. It also supports web sockets as maybe thats needed.

tons of game developers use nats to btw.

https://github.com/nats-io/nats-server

golang client: https://github.com/nats-io/nats.go

rust client: https://github.com/nats-io/nats.rs

the NATS server can store the screenshots and data using nats objj store and kv store if you just want to keep things simple.

using the api is really easy i find from rust and golang.

@LoipesMas
Copy link
Contributor Author

@Flone-dnb with certbot, you can set up free and automatic certificates for your https server. This shouldn't be much harder than setting up the server itself.
But having a certificate wouldn't really be a requirement. As you said, it's not really sensitive information and the requests could be sent over http (without the s).

So considering how simple and standard using https would be, there's not a lot of reasons not to. But on the other hand, it's also not a high-priority thing.

@Flone-dnb
Copy link
Owner

Flone-dnb commented May 22, 2023

@gedw99 You are right, NATS can actually be one of the solutions. Although I haven't used NATS myself, I know that some C++ game projects from my job use NATS. Unfortunately, I think NATS will be overkill for this project. I already wrote a very simple API for handling encrypted TCP messaging in the shared crate (including file transfer), it works fine and seems to not have any critical issues (although I think our file transfer or the fact that it does not use async will slow us down) now I'm just looking for a proper "battle-tested" simple / lightweight library for handing low-level TCP stuff for me.

@LoipesMas Hey thanks for the info, I didn't knew about certbot.

As you said, it's not really sensitive information and the requests could be sent over http (without the s).

I'm glad you can agree on this, although when I said:

I mean this is not some bank management application, bug reports don't hold any sensitive information

I meant that at least some (poor) encryption is required (we don't need super secured stuff here), moreover, I don't want to just throw away encryption that we currently have. I mean it's not perfect and probably has some security issues, but hey it's better that nothing. Additionally, although reports might not have much sensitive information, client login/passwords do have sensitive information. Imagine you are a developer and you want to check the report list, you connect to the server by sending there your login and password hash - I think this is sensitive information, sending this over HTTP is probably a bad idea.

So I think that we should at least stick with the web framework + certbot solution. Maybe we can detect if the server (built in the release mode) is started in the HTTP mode and just throw an error and don't start at all to prevent the users from shooting themselves in the foot. Yeah, this might work.

So OK, let's say our 2.0 goal is to:

  • rewrite the server to use salvo (because it seems to be super simple and I wanted to learn it 😄)
  • eliminate the client application and just have a web interface that the server provides

This will most likely happen in the next year (because I don't have time for this now and this is not a high priority). But don't think there won't be any updates this year. I already updated the project to support Godot 4 and I plan to try to add Github Issues support this/next week (see dev branch).

Any thumbs up/down on this?

@Flone-dnb Flone-dnb removed the question Further information is requested label May 22, 2023
@Flone-dnb Flone-dnb added this to the v2.0.0 milestone May 23, 2023
@LoipesMas
Copy link
Contributor Author

Web framework + certbot sounds good!
Haven't used salvo myself, but it should fit our needs.

Somewhere down the line you could consider selling servers as a service (setting up, maintenance, etc.), for those users who don't want to bother with that, in order to fund further development. But that's just a thought for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: hard Not suited for newcomers enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants