Broken live collaboration about excalidraw-room HOT 10 CLOSED

Thanks for the quick fix! 🙏

from excalidraw-room.

Looks like alerting would push the price from the current $7 a month to $25 a month.

from excalidraw-room.

first, I'm terribly sorry for breaking the app today.

I tried to reproduce the issue locally by running the frontend and the backend on different domains, and I can't reproduce it.
I'm running the backend repository with the changes from my PR, and all WebSocket requests sent to the backend come back with an access-control-allow-origin header with the value being the domain of the frontend app.
The value of the access-control-allow-origin header on the response is matching the origin header of the request.

I'm down to help fix it but I need to figure out a way to reproduce it first.

from excalidraw-room.

@asvinours no worries, I don't think it was your fault. We shouldn't be able to make the entire server crash by fixing something.

I am not familiar with how to run it or the app itself, @idlewinn, @lipis and @petehunt built it I believe. It would be nice if they could help you out debugging this.

from excalidraw-room.

Numbers are back looking healthy

from excalidraw-room.

Oh, my fault for not properly testing before merging.

Thing is, I'm running the exact same thing on a different server so I assumed it's safe to merge. Nvm, that server isn't actually subject to preflight requests. But I don't see any OPTIONS requests on excalidraw.com either. It's hard to debug this one.

Btw, the handlePreflightRequest is undocumented, and the typings are wrong. There's no Server argument, the signature should be (req: http.IncomingMessage, res: http.ServerResponse). I'm wondering how it could work before at all. EDIT: ok, that seems to be the problem as the latest PR introduced that change to function signature.

from excalidraw-room.

socketio/socket.io-client#1140 (comment)
socketio/socket.io-client#1140 (comment)

I think the second comment is pointing out the issue.
My IDE is telling me the signature of the function is the following: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/engine.io/index.d.ts#L107

from excalidraw-room.

I think the types are simply wrong. The 2.x.x and 3.x.x have the server as thisArg instead of an argument, and in 4.x.x the handlePreflightRequest() API is removed altogether.

from excalidraw-room.

We should add a test that makes sure that the build actually works somehow.

At the moment, Heroku itself picks up a new commit was merged and starts the deployment process. We can alter that to make a GitHub action do the deploy so we are able to run a test before that that will actually test the server is working before doing the deployment. We also have a staging app on Heroku that we can use to make an integration test but don't quote me on that, it may need a bit more investigation.

There is an issue already #13 that I'm suspecting it was meant to allow doing that but never picked up.

from excalidraw-room.

Should be ok now.. no?

from excalidraw-room.