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

http and web socket server in the same bot #21

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

voldyman
Copy link

No description provided.

@bradchoate
Copy link
Contributor

👎 I don't think this is necessary or desired. At least for this bot, the front-end is going to live on another service (caseyliss.com, or eventually atp.fm on Squarespace). There's no benefit to having a web server in-process.

@banksJeremy
Copy link
Contributor

I see your point. However, even if Casey doesn't use it in this way (and I think he said he was considering it until he ran into the port limitation), he was hopeful this bot it might be used by others. I could imagine a fairly large portion of users finding this kind of self-contained configuration convenient. (As a first, I find it convenient for testing my changes on Heroku.)

If we don't want the server in accidentalbot.js to support HTTP by default, what about having support in a webclient/server.js, which can be run instead to launch a combined Server? accidentalbot.js would need to be reorganized a bit to let it also add to an existing HTTP Server instead of just creating a new Server, so this would probably have to wait until current major changes have settled down.

@bradchoate
Copy link
Contributor

I can see the convenience for testing, but we're trying to harden this server and adding express opens up exploit avenues instead of reducing them.

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

Successfully merging this pull request may close these issues.

4 participants