Skip to content

Crystal 0.35 support#3

Merged
fernandes merged 6 commits into
cable-cr:masterfrom
jwoertink:crystal_0.35
Nov 21, 2020
Merged

Crystal 0.35 support#3
fernandes merged 6 commits into
cable-cr:masterfrom
jwoertink:crystal_0.35

Conversation

@jwoertink
Copy link
Copy Markdown
Collaborator

@jwoertink jwoertink commented Nov 12, 2020

Fixes #2

This PR lets the shard compile on Crystal 0.35. However, there's still a few issues to sort out.

  • - Specs fail locally (and on the CI apparently)
  • - The logger needs to be refactored to support the new logger
  • - ....
  • - Profit!

@jwoertink
Copy link
Copy Markdown
Collaborator Author

@fernandes I'm not sure what specs were broken before this PR. Would you like them fixed in this PR, or a separate one? Also same with the logger? Right now it's just deprecation warnings, but should they be fixed in this, or a separate PR?

request = HTTP::Request.new("GET", "/unknown_route?test_token=1", headers)

io_with_context = create_ws_request_and_return_io_and_context(handler, request)[0]
io_with_context.to_s.should eq("")
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is meant to be nothing, but when the server hits a route that doesn't exist, we get back the 404.

Comment thread src/cable/handler.cr Outdated

Cable::Logger.info "Successfully upgraded to WebSocket (REQUEST_METHOD: GET, HTTP_CONNECTION: Upgrade, HTTP_UPGRADE: websocket)"
content = ws.call(context)
content.as(Proc(IO, Nil)).call(context.response.output)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if this is needed, but without it, the websocket block is never called (at least on this Fiber). By adding it, the socket.send welcome is added to the response.

One other thing is that the type of ws.call(context) kept coming back as

In src/cable/handler.cr:48:21

 48 | raise content.call.inspect
                    ^---
Error: undefined method 'call' for Array(Log::Entry) (compile-time type is (Array(Log::Entry) | Bool | IO | Int32 | Int64 | Proc(IO, Nil) | Nil))

so I had to typecast it just to get this to run.

@jwoertink jwoertink mentioned this pull request Nov 16, 2020
@fernandes
Copy link
Copy Markdown
Collaborator

@jwoertink I added the fix from #9 and fixed CI, let's see if it's gonna be ✅ now!

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.

Crystal 0.35 support

2 participants