SimonDanisch / Bonito.jl

Serving JS to the browser
MIT License
204 stars 29 forks source link

Add `WebSocketHandler` helper for websocket-based FrontendConnection extensions #247

Closed frankier closed 1 week ago

frankier commented 1 month ago

Previously outlined here: https://github.com/SimonDanisch/Bonito.jl/issues/244

frankier commented 4 weeks ago

@SimonDanisch Could you please approve my workflows? :o)

frankier commented 3 weeks ago

I think the CI is timing out, but it does not appear to obviously be related to the content of the PR.

Looks like the documentation preview wasn't built because the PR is from a fork.

SimonDanisch commented 3 weeks ago

It does seem to be related?

Warning: error while cleaning up server
│   exception =
│    MethodError: no method matching isopen(::Nothing)
│    Closest candidates are:
│      isopen(!Matched::Union{Base.AsyncCondition, Timer}) at asyncevent.jl:138
│      isopen(!Matched::Base.AbstractPipe) at io.jl:385
│      isopen(!Matched::HTTP.Servers.Server) at /home/runner/.julia/packages/HTTP/sJD5V/src/Servers.jl:127
│      ...
│    Stacktrace:
│     [1] isopen(ws::WebSocketConnection)
│       @ Bonito ~/work/Bonito.jl/Bonito.jl/src/connection/websocket.jl:14
│     [2] isopen(session::Session{WebSocketConnection})
│       @ Bonito ~/work/Bonito.jl/Bonito.jl/src/session.jl:162
│     [3] (::Bonito.var"#100#101"{Server, Set{WebSocketConnection}})()
│       @ Bonito ~/work/Bonito.jl/Bonito.jl/src/connection/websocket.jl:87
│     [4] lock(f::Bonito.var"#100#101"{Server, Set{WebSocketConnection}}, l::ReentrantLock)
│       @ Base ./lock.jl:187
│     [5] cleanup_server
│       @ ~/work/Bonito.jl/Bonito.jl/src/connection/websocket.jl:75 [inlined]
│     [6] macro expansion
│       @ ~/work/Bonito.jl/Bonito.jl/src/connection/websocket.jl:112 [inlined]
│     [7] (::Bonito.var"#103#105"{Server})()
│       @ Bonito ./task.jl:417
└ @ Bonito ~/work/Bonito.jl/Bonito.jl/src/connection/websocket.jl:115
frankier commented 3 weeks ago

Ah. Looks like I pushed a slightly different version from the one I had manually tested. For some reason. I have trouble running the tests locally (they seem to pop up a few windows and then freeze indefinitely).

As for the CI, when I inspect the output it looks like it's timed out after 6 hours and all of the output after the first 50 lines (featuring package installs) is blank so I don't see the error you pasted.

frankier commented 3 weeks ago

I'll wait to see how https://github.com/SimonDanisch/Bonito.jl/pull/248 gets fixed and then rebase + try to fix this.

frankier commented 1 week ago

Figured out the problem -- it was rather simple and but not fully obvious due to exceptions not printing in sub threads -- it should work now. I was able to run the tests locally using xvfb-run.

Can still hold off until https://github.com/SimonDanisch/Bonito.jl/pull/251 if you like

SimonDanisch commented 1 week ago

it was rather simple and but not fully obvious due to exceptions not printing in sub threads

I did run into this as well... Not really clear why though -.- I suspect apply_handler to suck up exceptions in HTTP.jl, so maybe we need a try catch there to show them ourselves.

SimonDanisch commented 1 week ago

Thank you :)