compatible with HTTP [1.1.0 - 1.6.0) #182

anj00 · 2022-11-19T16:43:23Z

I tried to update to HTTP 1.+

I could only make tests running for HTTP 1.1.0 and 1.5.5 (I could imagine all inbetween will be running). And of course this code will not run with HTTP prior 1.1.0
bumped WebSockets version to 1.6.0 as this is pretty major move
some test somehow needed a sleep(1) to actually run.
copied some (basic skeletons) functions from HTTP 0.9 to HTTP.jl

…t LTS and latest released

anj00 · 2022-11-19T17:35:00Z

I tried to update to HTTP 1.+

I could only make tests running for HTTP 1.1.0 and 1.5.5 (I could imagine all inbetween will be running). And of course this code will not run with HTTP prior 1.1.0
bumped WebSockets version to 1.6.0 as this is pretty major move
some test somehow needed a sleep(1) to actually run.
copied some (basic skeletons) functions from HTTP 0.9 to HTTP.jl
bump julia version to 1.6 min (same as for HTTP)
appveyor somehow has a bug and tests against 1.5 julia. explicitly put LTS and latest released

hustf · 2022-11-19T17:48:25Z

test/client_serverWS_test.jl

@@ -50,13 +51,15 @@ let
    WebSockets.open(initiatingws, "ws://$SURL:$PORT")
    close(server)
 end
+sleep(1)


Move inside let block? We should not be surprised that a 'sleep' is required. 'yield' would probably be sufficient and not take extra time.

hustf · 2022-11-19T17:53:55Z

test/client_server_functions.jl

@@ -134,7 +139,7 @@ function initiatingws(ws::WebSocket; msglengths = MSGLENGTHS, closebeforeexit =
 end

 test_serverws = WebSockets.ServerWS(
-    HTTP.RequestHandlerFunction(test_handler),
+    WebSockets.RequestHandlerFunction(test_handler),


I like the change. Too many things were called prefixed HTTP. I would also think it's OK to export RequestHandlerFunction and similar, and refer to them wihout module prefix here in the tests.

anj00 · 2022-11-19T21:38:18Z

I guess it needs more work. It does hang in tests (works here locally). So something is not right. If anyone can take a look or has an idea, that would be great

mind6 · 2022-11-20T17:15:56Z

test/show_test.jl

@@ -6,7 +6,7 @@ end

 let kws = [], msgs =[]
    ds = DummyStream(IOBuffer(), 0, 0)
-    for s = 0:9, h in [Base.C_NULL, Ptr{UInt64}(3)]
+    for s = 0:9, h in [Base.C_NULL, Ptr{Cvoid}(3)]


I'm just making a note as I study your changes.
WebSockets uses Base.LibuvStream, which depends on Base.LibuvServer, which specifically requires handle to be of type Ptr{Cvoid} after Julia 1.6. Cvoid is aliased to the Nothing type.

mind6 · 2022-11-20T17:17:53Z

test/show_test.jl

@@ -222,7 +222,7 @@ let chnlout, sws, sws1, sws2
    io = IOBuffer()
    show(io, sws)
    output = String(take!(io))
-    @test output == "WebSockets.ServerWS(handler=h(r), wshandler=w(s)).out:Channel{Any}(sz_max:2,sz_curr:2) "
+    @test output == "WebSockets.ServerWS(handler=h(r), wshandler=w(s)).out:Channel{Any}(2) "


Base.Channel no longer maintains a sz_curr field, so it only displays sz_max without naming it.

mind6 · 2022-11-20T17:30:18Z

So the CI tests are hanging on 32-bit x86, and Julia 1.8.3 and nightly?
I will make a mini-PR with only show-test.jl modified, to see if the original version pass these tests

anj00 · 2022-11-21T15:37:33Z

Well, the problem looks to be on trying to do anything with the socket

readbytes!(ws.socket, ab) hangs
I tried to use peek(ws.socket) before calling readbytes!. But peek hangs as well now.

Maybe there are other methods to understand that the socket is actually ready?

And the issue seem to be happening only here
@info "Listen: Server side initiates message exchange."
@info "Listen: Server side initiates message exchange. Close from within server side handler."

i.e. the tests there echows function is used.

mind6 · 2022-11-21T16:02:38Z

Well, the problem looks to be on trying to do anything with the socket

readbytes!(ws.socket, ab) hangs I tried to use peek(ws.socket) before calling readbytes!. But peek hangs as well now.

Maybe there are other methods to understand that the socket is actually ready?

And the issue seem to be happening only here @info "Listen: Server side initiates message exchange." @info "Listen: Server side initiates message exchange. Close from within server side handler."

i.e. the tests there echows function is used.

In HTTP 0.9, the CI tests behave the same way they do on my Windows 10 machine. Are your tests passing locally? Are you on Linux?

anj00 · 2022-11-21T16:39:46Z

tests pass locally
I am on windows 10.

mind6 · 2022-11-21T17:34:07Z

tests pass locally I am on windows 10.

I wonder if HTTP 1.5 tests pass in CI, if you just made a spurious pull request to HTTP.jl 😆

anj00 · 2022-11-21T19:37:05Z

I only added a couple of printouts and now tests are good.

anj00 · 2022-11-22T10:16:01Z

Can someone with right access please comment on the changes? Accept the changes? So WebSockets can move on HTTP 1.5 and stop blocking other packages?

mind6 · 2022-11-22T13:29:47Z

Can someone with right access please comment on the changes? Accept the changes? So WebSockets can move on HTTP 1.5 and stop blocking other packages?

I think he's on vacation for a few days

hustf · 2022-11-23T20:11:31Z

Vocation this time... Tomorrow is WebSockets fun time. Looking forward to see how you solved this! Thanks specifically for pulling this through CI, which is sometimes arbitrary, sometimes evil.

hustf

An excerpt from local tests, run by ]test in a Websockets.jl folder.
(last edit: some are fixed by simply running Julia 1.8.3 instead of 1.8.1. Summaries at the end of this comment.)

Some of the errors are typical for the context switch between ]test and include("test/runtests.jl"). In produced strings, the module reference to a function often changes. The fix is easy: When testing strings, test those produced by both contexts.

1)

Local failure on Julia 1.8.1 Win64 after commit 491463b:

 Test Failed at C:\Users\frohu_h4g8g6y\.julia\environments\webs\WebSockets.jl\test\test_websocketlogger.jl:59
  Expression: Logging.default_metafmt(Error, Main, :g, :i, "", 0) == (:light_red, "Error:", "@ Main :0")
   Evaluated: (:yellow, "Error:", "@ Main :0") == (:light_red, "Error:", "@ Main :0")

2)

Local error on Julia 1.8.1 Win64 after commit 491463b. The rest of the tests in this file may not have been run.

Client: Error During Test at C:\Users\frohu_h4g8g6y\.julia\environments\webs\WebSockets.jl\test\runtests.jl:32
  Got exception outside of a @test
  LoadError: HTTP.Exceptions.RequestError(HTTP.Messages.Request:
  """
  GET / HTTP/1.1
  Upgrade: websocket
  Connection: Upgrade
  Sec-WebSocket-Key: O3opyqR/AttF7EbohEmhaw==
  Sec-WebSocket-Version: 13
  Host: 127.0.0.1:8091
  Accept: */*
  User-Agent: HTTP.jl/1.8.1
  Accept-Encoding: gzip

  [Message Body was streamed]""", Base.IOError("read: connection reset by peer (ECONNRESET)", -4077))
  Stacktrace:
...
   [15] open(f::Function, url::String; verbose::Bool, subprotocol::String, kw::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
      @ WebSockets C:\Users\frohu_h4g8g6y\.julia\environments\webs\WebSockets.jl\src\HTTP.jl:89
   [16] open(f::Function, url::String)
      @ WebSockets C:\Users\frohu_h4g8g6y\.julia\environments\webs\WebSockets.jl\src\HTTP.jl:68
   [17] top-level scope
      @ C:\Users\frohu_h4g8g6y\.julia\environments\webs\WebSockets.jl\test\client_test.jl:23
...
  caused by: IOError: read: connection reset by peer (ECONNRESET)

3)

Local error on Julia 1.8.1 Win64 after commit 491463b. The rest of the tests in this file may not have been run.

 Error During Test at C:\Users\frohu_h4g8g6y\.julia\environments\webs\WebSockets.jl\test\runtests.jl:42
  Got exception outside of a @test
  LoadError: HTTP.Exceptions.RequestError(HTTP.Messages.Request:
  """
  GET / HTTP/1.1
  Upgrade: websocket
  Connection: Upgrade
  Sec-WebSocket-Key: lr7R/fs9tZGNeXuKY/XpOg==
  Sec-WebSocket-Version: 13
  Host: 127.0.0.1:8000
  Accept: */*
  User-Agent: HTTP.jl/1.8.1
  Accept-Encoding: gzip

  [Message Body was streamed]""", Base.IOError("read: connection reset by peer (ECONNRESET)", -4077))
  Stacktrace:
...
   [15] open(f::Function, url::String; verbose::Bool, subprotocol::String, kw::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
      @ WebSockets C:\Users\frohu_h4g8g6y\.julia\environments\webs\WebSockets.jl\src\HTTP.jl:89
   [16] open(f::Function, url::String)
      @ WebSockets C:\Users\frohu_h4g8g6y\.julia\environments\webs\WebSockets.jl\src\HTTP.jl:68
   [17] top-level scope
      @ C:\Users\frohu_h4g8g6y\.julia\environments\webs\WebSockets.jl\test\client_serverWS_test.jl:50
...
  caused by: IOError: read: connection reset by peer (ECONNRESET)

4)

Local failure on Julia 1.8.1 Win64 after commit 491463b.

Abrupt close & error handling: Test Failed at C:\Users\frohu_h4g8g6y\.julia\environments\webs\WebSockets.jl\test\error_test.jl:118
  Expression: err.message == "while read(ws|server) Client side closed socket connection - Performed closing handshake."   Evaluated: "while read(ws|server) Base.IOError(\"read: connection reset by peer (ECONNRESET)\", -4077)" == "while read(ws|server) Client side closed socket connection - Performed closing handshake."

5)

Local failure on Julia 1.8.1 Win64 after commit 491463b.

Abrupt close & error handling: Test Failed at C:\Users\frohu_h4g8g6y\.julia\environments\webs\WebSockets.jl\test\error_test.jl:146
  Expression: err.message == "while read(ws|server) Client side closed socket connection - Performed closing handshake."   Evaluated: "while read(ws|server) Base.IOError(\"read: connection reset by peer (ECONNRESET)\", -4077)" == "while read(ws|server) Client side closed socket connection - Performed closing handshake."

##############################

Summary from local ]test at Julia 1.8.1:

##############################

Test Summary:                              | Pass  Fail  Error  Total     Time
WebSockets                                 | 1015     3      2   1020  2m09.7s
  Base.show                                |   33                  33     2.3s
  WebSocketLogger                          |   56     1            57     4.5s
    WebSocketLogger                        |   35     1            36     2.8s
      Default metadata formatting          |    8     1             9     0.4s
      Prefix and suffix layout             |    4                   4     0.1s
      Metadata suffix, right justification |    7                   7     0.0s
      Limiting large data structures       |    2                   2     0.7s
    WebSocketLogger_special                |   21                  21     0.6s
  Fragment and unit                        |  823                 823    22.7s
  Handshake                                |   27                  27     2.0s
  Client                                   |                 1      1    10.6s
  Client_listen                            |   18                  18     5.5s
  Client_serverWS                          |   10            1     11     1.0s
  Abrupt close & error handling            |   48     2            50  1m20.5s
ERROR: LoadError: Some tests did not pass: 1015 passed, 3 failed, 2 errored, 0 broken.

#################################

Summary from include("test/runtests.jl") at Julia 1.8.1:

#################################

Test Summary:                              | Pass  Fail  Error  Total     Time
WebSockets                                 | 1025     3      1   1029  2m09.1s
  Base.show                                |   33                  33     2.3s
  WebSocketLogger                          |   56     1            57     4.4s
    WebSocketLogger                        |   35     1            36     2.6s
      Default metadata formatting          |    8     1             9     0.4s
      Prefix and suffix layout             |    4                   4     0.1s
      Metadata suffix, right justification |    7                   7     0.0s
      Limiting large data structures       |    2                   2     0.6s
    WebSocketLogger_special                |   21                  21     0.6s
  Fragment and unit                        |  823                 823    22.6s
  Handshake                                |   27                  27     2.0s
  Client                                   |                 1      1     9.7s
  Client_listen                            |   18                  18     5.1s
  Client_serverWS                          |   20                  20     2.4s
  Abrupt close & error handling            |   48     2            50  1m20.3s
ERROR: LoadError: Some tests did not pass: 1025 passed, 3 failed, 1 errored, 0 broken.

#################################

Summary from include("test/runtests.jl") at Julia 1.8.3:

#################################

Test Summary:                              | Pass  Fail  Total     Time
WebSockets                                 | 1040     1   1041  3m24.2s
  Base.show                                |   33           33     2.3s
  WebSocketLogger                          |   56     1     57     4.4s
    WebSocketLogger                        |   35     1     36     2.7s
      Default metadata formatting          |    8     1      9     0.4s
      Prefix and suffix layout             |    4            4     0.1s
      Metadata suffix, right justification |    7            7     0.0s
      Limiting large data structures       |    2            2     0.6s
    WebSocketLogger_special                |   21           21     0.6s
  Fragment and unit                        |  823          823    22.7s
  Handshake                                |   27           27     2.0s
  Client                                   |   13           13  1m05.6s
  Client_listen                            |   18           18     4.9s
  Client_serverWS                          |   20           20     2.4s
  Abrupt close & error handling            |   50           50  1m39.5s
ERROR: LoadError: Some tests did not pass: 1040 passed, 1 failed, 0 errored, 0 broken.

##############################

Summary from local ]test at Julia 1.8.3:

##############################
I now wish I had started with this version!
The one reported 'failure' is probably due to environmental variables set in my console script, where I tweaked the colors to have clearer error messages!
This is surprising. We need to consider if we wish Julia 1.8.3 as a minimum version for this coming version of WebSockets. Early failures are preferable in my book. I doubt if there are other packages which require 1.8 to 1.8.2.

Test Summary:                              | Pass  Fail  Total     Time
WebSockets                                 | 1040     1   1041  3m24.5s
  Base.show                                |   33           33     2.4s
  WebSocketLogger                          |   56     1     57     4.6s
    WebSocketLogger                        |   35     1     36     2.8s
      Default metadata formatting          |    8     1      9     0.4s
      Prefix and suffix layout             |    4            4     0.1s
      Metadata suffix, right justification |    7            7     0.0s
      Limiting large data structures       |    2            2     0.7s
    WebSocketLogger_special                |   21           21     0.6s
  Fragment and unit                        |  823          823    22.7s
  Handshake                                |   27           27     2.0s
  Client                                   |   13           13  1m05.8s
  Client_listen                            |   18           18     4.9s
  Client_serverWS                          |   20           20     2.4s
  Abrupt close & error handling            |   50           50  1m39.4s
ERROR: LoadError: Some tests did not pass: 1040 passed, 1 failed, 0 errored, 0 broken.

hustf · 2022-11-24T09:57:17Z

Regarding test output to console: There's been a small explosion in the number of outputs. A lot of this is clutter from lower-level functionality, since the package HTTP now outputs debug statements in a different manner from before. We should have a look at cleaning up the clutter (through modifying 'shouldlog'), but for now it's probably better to keep this PR minimal.

fonsp · 2022-11-24T10:23:14Z

@hustf Awesome to see that this is getting picked up again! If you are interested, I would love to have a call about the position of WebSockets.jl after the recent overhaul of HTTP.WebSockets: JuliaWeb/HTTP.jl#843 . It would be great if we could centralise documentation and issues. You can reach me at: fons@plutojl.org

hustf · 2022-11-24T10:53:16Z

Updated the long comment above.

For the open process: Should we exclude Julia 1.8.0 to 1.8.2? Would lazy updaters benefit from downloading a new version of WebSockets with deprecation or warning messages? Note that I have not checked if 1.8.2 would fully work.

Also note that since this PR seems to be working well, I'll also have a look and smell at the code before merging.

mind6 · 2022-11-24T13:08:01Z

Abrupt close & error handling: Test Failed at C:\Users\frohu_h4g8g6y\.julia\environments\webs\WebSockets.jl\test\error_test.jl:118
  Expression: err.message == "while read(ws|server) Client side closed socket connection - Performed closing handshake."   Evaluated: "while read(ws|server) Base.IOError(\"read: connection reset by peer (ECONNRESET)\", -4077)" == "while read(ws|server) Client side closed socket connection - Performed closing handshake."

These two errors look like the two errors when using HTTP 0.9

So far it's been oscillating a bit.

Julia Ver.	lines 118,146 tests
1.6.3	pass
1.6.7	fail
1.8.1	fail
1.8.2	pass
1.8.3	pass
nightly	pass

Well the trend is good. We can call it a pass if we want. 😁

p.s. I think the people wanting to update, want the latest packages with HTTP on latest Julia. Requiring 1.8.3 is fine if that is truly stable. Someone still on LTS may not care about HTTP 1.

anj00 · 2022-11-25T11:48:53Z

I realized that handler function interface for HTTP was changed for no good reason. so rolled it back to original and moved details inside the module. So users would have easier time migrating.

I believe the migration is only going from this

WebSockets.ServerWS(
    HTTP.RequestHandlerFunction(test_handler),
    WebSockets.WSHandlerFunction(test_wshandler))

To this

WebSockets.ServerWS(
    WebSockets.RequestHandlerFunction(test_handler),
    WebSockets.WSHandlerFunction(test_wshandler))

Probably needs to be put in readme?``

Project.toml

hustf · 2022-11-25T18:40:17Z

After commit fc55503, examples/chat_explore.jl works again. So that syntax is still OK.

But README.md and Project.toml needs an update, possibly other files do as well.

I will call this PR successful and merge it to master, so that it will be easier for @mind6 to contribute, and people who are not interested in websockets per se can checkout master. This is no doubt an improvement over the current master's state.

Also, when CI is run from Websockets master, I believe the job will be submitted with different ownership. I was not able to cancel and restart hung jobs from this PR. Probably, anj00 has that access too.

Oh, and by the way: Thank you very much!

hustf · 2022-11-29T11:28:07Z

I believe the migration is only going from this...

There were two issues with re-running the old README example.

WebSocketLogger was not in scope. The error message was captured by HTTP.jl somewhere, as the with_logger call was compiled.
HTTP no longer accepted Response(200) as valid. Response(200, OK) works. Instead of posting an issue with HTTP.jl, I went ahead and updated our code.

andrei added 3 commits November 19, 2022 17:34

compatible with HTTP [1.1.0 - 1.6.0)

8bbde79

bump julia version to 1.6 min

e736307

appveyor somehow has a bug and tests against 1.5 julia. explicitly pu…

8ebc385

…t LTS and latest released

hustf mentioned this pull request Nov 19, 2022

Update to HTTP 1? #181

Closed

hustf reviewed Nov 19, 2022

View reviewed changes

andrei added 2 commits November 19, 2022 19:04

move sleep inside let block

9e39d2e

adding more sleep

ad81260

mind6 reviewed Nov 20, 2022

View reviewed changes

andrei added 4 commits November 20, 2022 18:58

explicitly wait for servertask to finish

53e1071

more debug about echo function

bd080c2

experiment with non blocking read to debug test fail in CI

9a4a378

oeek cocket before blocking read

fc05f3c

andrei added 3 commits November 21, 2022 18:34

debug around server code

b36bccd

more logging on the date write/read

a410ecc

remova read_frame debug code

491463b

hustf reviewed Nov 24, 2022

View reviewed changes

test_handler is back to original signature deals with request not stream

fc55503

hustf reviewed Nov 25, 2022

View reviewed changes

Project.toml Show resolved Hide resolved

hustf merged commit 69c582d into JuliaWeb:master Nov 25, 2022

johnnychen94 mentioned this pull request Nov 27, 2022

Make Pluto dependency optional? JuliaDocs/DemoCards.jl#130

Closed

anj00 mentioned this pull request Dec 10, 2022

remove dependency on WebSockets JuliaGizmos/Blink.jl#295

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

compatible with HTTP [1.1.0 - 1.6.0) #182

compatible with HTTP [1.1.0 - 1.6.0) #182

anj00 commented Nov 19, 2022 •

edited

Loading

anj00 commented Nov 19, 2022

hustf Nov 19, 2022

hustf Nov 19, 2022

anj00 commented Nov 19, 2022

mind6 Nov 20, 2022 •

edited

Loading

mind6 Nov 20, 2022

mind6 commented Nov 20, 2022

anj00 commented Nov 21, 2022 •

edited

Loading

mind6 commented Nov 21, 2022

anj00 commented Nov 21, 2022

mind6 commented Nov 21, 2022

anj00 commented Nov 21, 2022

anj00 commented Nov 22, 2022

mind6 commented Nov 22, 2022

hustf commented Nov 23, 2022

hustf left a comment •

edited

Loading

hustf commented Nov 24, 2022

fonsp commented Nov 24, 2022

hustf commented Nov 24, 2022

mind6 commented Nov 24, 2022 •

edited

Loading

anj00 commented Nov 25, 2022

hustf commented Nov 25, 2022 •

edited

Loading

hustf commented Nov 29, 2022

compatible with HTTP [1.1.0 - 1.6.0) #182

compatible with HTTP [1.1.0 - 1.6.0) #182

Conversation

anj00 commented Nov 19, 2022 • edited Loading

anj00 commented Nov 19, 2022

hustf Nov 19, 2022

Choose a reason for hiding this comment

hustf Nov 19, 2022

Choose a reason for hiding this comment

anj00 commented Nov 19, 2022

mind6 Nov 20, 2022 • edited Loading

Choose a reason for hiding this comment

mind6 Nov 20, 2022

Choose a reason for hiding this comment

mind6 commented Nov 20, 2022

anj00 commented Nov 21, 2022 • edited Loading

mind6 commented Nov 21, 2022

anj00 commented Nov 21, 2022

mind6 commented Nov 21, 2022

anj00 commented Nov 21, 2022

anj00 commented Nov 22, 2022

mind6 commented Nov 22, 2022

hustf commented Nov 23, 2022

hustf left a comment • edited Loading

Choose a reason for hiding this comment

1)

2)

3)

4)

5)

Summary from local ]test at Julia 1.8.1:

Summary from include("test/runtests.jl") at Julia 1.8.1:

Summary from include("test/runtests.jl") at Julia 1.8.3:

Summary from local ]test at Julia 1.8.3:

hustf commented Nov 24, 2022

fonsp commented Nov 24, 2022

hustf commented Nov 24, 2022

mind6 commented Nov 24, 2022 • edited Loading

anj00 commented Nov 25, 2022

hustf commented Nov 25, 2022 • edited Loading

hustf commented Nov 29, 2022

anj00 commented Nov 19, 2022 •

edited

Loading

mind6 Nov 20, 2022 •

edited

Loading

anj00 commented Nov 21, 2022 •

edited

Loading

hustf left a comment •

edited

Loading

mind6 commented Nov 24, 2022 •

edited

Loading

hustf commented Nov 25, 2022 •

edited

Loading