Skip to content

Commit

Permalink
Handle redirection manually.
Browse files Browse the repository at this point in the history
When keep-alive is enabled and the redirection is cross-origin, em-http-request create a new connection,
thus making the callbacks non-operent.
  • Loading branch information
francois2metz committed Jun 3, 2018
1 parent 52884bd commit d5a512d
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 17 deletions.
14 changes: 9 additions & 5 deletions lib/em-eventsource.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ class EventSource
attr_reader :inactivity_timeout
# Set the inactivity timeout
attr_writer :inactivity_timeout
# Set the max_redirects
attr_writer :max_redirects
# Ready state
# The connection has not yet been established, or it was closed and the user agent is reconnecting.
CONNECTING = 0
Expand All @@ -45,7 +43,6 @@ def initialize(url, query={}, headers={})
@last_event_id = nil
@retry = 3 # seconds
@inactivity_timeout = 60 # seconds
@max_redirects = 5

@opens = []
@errors = []
Expand Down Expand Up @@ -130,6 +127,13 @@ def listen

def handle_reconnect(*args)
return if @ready_state == CLOSED

if @req.response_header.redirection?
@url = @req.response_header.location
listen
return
end

@ready_state = CONNECTING
@errors.each { |error| error.call("Connection lost. Reconnecting.") }
EM.add_timer(@retry) do
Expand All @@ -139,6 +143,7 @@ def handle_reconnect(*args)

def handle_headers(headers)
if headers.redirection?
@req.close
return
end
if headers.status != 200
Expand Down Expand Up @@ -196,9 +201,8 @@ def prepare_request
headers = @headers.merge({'Cache-Control' => 'no-cache', 'Accept' => 'text/event-stream'})
headers.merge!({'Last-Event-Id' => @last_event_id }) if not @last_event_id.nil?
[conn, conn.get({ query: @query,
redirects: @max_redirects,
head: headers,
keepalive: true})]
keepalive: true })]
end
end
end
25 changes: 16 additions & 9 deletions spec/em-eventsource_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ def create_response_headers(status, content_type="", other={})
req.get_args[0].must_equal({ query: {},
head: {"Cache-Control" => "no-cache",
"Accept" => "text/event-stream"},
keepalive: true,
redirects: 5 })
keepalive: true })
EM.stop
end
end
Expand All @@ -45,8 +44,7 @@ def create_response_headers(status, content_type="", other={})
head: {"DNT" => 1,
"Cache-Control" => "no-cache",
"Accept" => "text/event-stream"},
keepalive: true,
redirects: 5 })
keepalive: true })
EM.stop
end
end
Expand Down Expand Up @@ -107,8 +105,17 @@ def create_response_headers(status, content_type="", other={})
assert true
EM.stop
end
req.call_headers(create_response_headers "302", "", {'Location' => "http://example.com/streaming2"})
req.call_headers(create_response_headers "302", "", {'LOCATION' => "http://example.com/streaming2"})
assert req.closed
req.call_callback
req2 = source.instance_variable_get "@req"
refute_same(req2, req)
req2.get_args[0].must_equal({ head: { "Accept" => "text/event-stream",
"Cache-Control" => "no-cache" },
query: {},
keepalive: true })
req.call_headers(create_response_headers "200", "text/event-stream; charset=utf-8")
source.url.must_equal "http://example.com/streaming2"
end
end

Expand Down Expand Up @@ -160,6 +167,7 @@ def create_response_headers(status, content_type="", other={})

it "reconnect after error with last-event-id" do
start_source do |source, req|
req.call_headers(create_response_headers "200", "text/event-stream; charset=utf-8")
req.stream_data("id: roger#{eol}#{eol}")
source.error do |error|
error.must_equal "Connection lost. Reconnecting."
Expand All @@ -172,8 +180,7 @@ def create_response_headers(status, content_type="", other={})
"Accept" => "text/event-stream",
"Cache-Control" => "no-cache" },
query: {},
keepalive: true,
redirects: 5})
keepalive: true })
EM.stop
end
end
Expand All @@ -183,6 +190,7 @@ def create_response_headers(status, content_type="", other={})

it "reconnect after callback with last-event-id" do
start_source do |source, req|
req.call_headers(create_response_headers "200", "text/event-stream; charset=utf-8")
req.stream_data("id: roger#{eol}#{eol}")
source.error do |error|
error.must_equal "Connection lost. Reconnecting."
Expand All @@ -195,8 +203,7 @@ def create_response_headers(status, content_type="", other={})
"Accept" => "text/event-stream",
"Cache-Control" => "no-cache" },
query: {},
keepalive: true,
redirects: 5 })
keepalive: true })
EM.stop
end
end
Expand Down
9 changes: 6 additions & 3 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

module EventMachine
class MockHttpRequest
attr_reader :url, :get_args, :middlewares, :opts
attr_reader :url, :get_args, :middlewares, :opts, :response_header, :closed

def initialize(url, opts={})
@url = url
Expand All @@ -12,6 +12,8 @@ def initialize(url, opts={})
@headers = []
@middlewares = []
@opts = opts
@response_header = HttpResponseHeader.new({})
@closed = false
end

def get(*args)
Expand Down Expand Up @@ -52,11 +54,12 @@ def call_callback
end

def call_headers(headers)
@response_header = headers
@headers.each { |header| header.call(headers) }
end

def close(reason)
# do nothing
def close(reason="")
@closed = true
end
end

Expand Down

0 comments on commit d5a512d

Please sign in to comment.