Socket upgrade to V4 with prometheus support#24
Conversation
src/index.ts
Outdated
| cors: { | ||
| allowedHeaders: ["Content-Type", "Authorization"], | ||
| origin: process.env.CORS_ORIGIN || "*", | ||
| credentials: true, |
There was a problem hiding this comment.
What does this credentials imply?
There was a problem hiding this comment.
it's for authentication cookies, I removed it
src/index.ts
Outdated
| serverDebug(`${clientSocket} has left the ${roomID} room because the user limit was reached.`); | ||
| clientSocket.leave(roomID); | ||
| }); | ||
| const isFollowRoom = roomID.startsWith("follow@"); |
There was a problem hiding this comment.
this will be of use to newer excalidraw version when we upgrade it
src/index.ts
Outdated
| }); | ||
| }); | ||
| }); | ||
| } catch (error) { |
There was a problem hiding this comment.
No need to surroung all that in a try-catch, we want the Node process to abort if setting up the socket fails.
src/index.ts
Outdated
| cors: { | ||
| allowedHeaders: ["Content-Type", "Authorization"], | ||
| origin: process.env.CORS_ORIGIN || "*", | ||
| credentials: true, |
|
@saghul, backend is working fine with Jitsi meet, I have tested it locally |
|
Great news! Can you please add some simple CI which runs the linter and checks that it builds succesfully? |
|
Got it! I haven’t set up CI before, but I’ll definitely work on adding them, Will reach out if I need help |
|
@saghul Can you review the changes |
|
Can you please use the latest Node LTS? That would be 22. |
|
Any updates on this? |
|
Yeah, PR is ready to merge |
|
@mihhu Any objections? |
|
We should also consider updating the Dockerfile for the Docker Hub publishing workflow. |
|
@GoliathLabs Good point! @yashop7 could you please do that? |
|
Since we're here, it might be a good idea to specify a UID in the Dockerfile to run the container as, which would improve security. It could look smth like this |
|
@saghul should be ready to merge now |
|
Hey - there has no new release on the github page yet with this changes, could you please initialize that? |
|
Before creating a new release, I've created a "chore PR": #25. |
|
Sorry to bother again but why exactly got the "USER_LIMIT" Feature got removed in this pull from the 5edc587 commit? |
|
That must have been an oversight. Good catch! We'll need it back. |
|
I'll make a separate PR for this issue |
|
Somehow it is still "working" from jitsi side, because of this code: Just for your information |
4 participants
Prometheus Working Fine
