EM::EventSource blocks forever in tests after requiring goliath/test_helper

[aenain]

Hi!

Great work on goliath! 👍

I have encountered an issue regarding em-synchrony/em-http and gem em-eventsource. I want to test goliath api that serves an event source to stream updates from background workers using redis pubsub.

The thing is that by requiring goliath/test_helper em-synchrony/em-http is required and that makes http requests fiber-aware. Therefore spec wouldn't work without event_source_use_async_http!, because source.start is no longer synchronous call.

# events_app.rb
require 'goliath'
require 'redis'
require 'hiredis'
require 'em-hiredis'
require 'em-synchrony'

class EventsApp < Goliath::API
  def response(env)
    EM.synchrony do
      @redis = Redis.new(driver: :synchrony)
      @redis.psubscribe(channels(env)) do |on|
        on.pmessage do |pattern, event, data|
          event_name = event.sub(/^app:\d+:/, '')
          env.stream_send(payload(event: event_name, data: data))
        end
      end
    end

    streaming_response(
      200,
      'Content-Type' => 'text/event-stream',
      'X-Stream' => 'Goliath',
      'Access-Control-Allow-Origin' => '*',
    )
  end

  def on_close(env)
    @redis.punsubscribe(channels(env)) if @redis
  end

  private

  def channels(env)
    app_id = env['REQUEST_PATH']
      .slice(/\/applications\/(?<app_id>\d+)/, :app_id)

    %W(app:#{app_id}:* build:* heartbeat)
  end

  def payload(event:, data:)
    "event: #{event}\n" +
    "data: #{data}" +
    "\n\n"
  end
end

# spec/events_app_spec.rb
require 'json'
require 'redis'
require 'em-eventsource'
require 'goliath/test_helper'

Goliath.env = :test

require_relative '../events_app'

RSpec.describe 'EventsApp' do
  include Goliath::TestHelper

  def event_source_use_async_http!
    klass = EM::HttpConnection
    if klass.instance_methods.include?(:aget) # by em-synchrony/em-http
      original, async = %i(get aget).map { |m| klass.instance_method(m) }
      allow_any_instance_of(klass).to receive(:get) do |conn, *args|
        method = caller.join =~ /em-eventsource/ ? async : original
        method.bind(conn).call(*args)
      end
    end
  end

  before(:each) { event_source_use_async_http! }

  it 'streams events from redis' do
    messages = []
    content = { 'id' => 13 }.to_json

    with_api(EventsApp, {}) do |server|
      source = EM::EventSource.new("http://#{server.address}:#{server.port}/events/applications/1")
      source.on('update') { |msg| messages << msg }
      source.start

      EM.add_timer(1) do
        redis = EM::Hiredis.connect
        redis.publish('app:1:update', content)
      end

      EM.add_timer(2) { EM.stop }
    end

    expect(messages).to include(content)
  end
end

I see a few possible approaches to solve this and want to ask you what your thoughts are on the matter.

1. Adding a similar helper to goliath/test_helper_ws which would hide the nitty-gritty details inside.
2. Breaking up EM::EventSource#prepare_request to be able to separate making an actual request (using EM::HttpRequest#get) to a separate method and later on overriding it from EM::Synchrony::EventSource.
3. Changing EM::EventSource#prepare_request to check if the EM::HttpRequest#aget method is available and if it is, use it instead of EM::HttpRequest#get.
4. I was thinking about making lighter version of goliath/test_helper that will only define with_api, but it would make it even more brittle if one day someone tries to test both sse and regular requests in one suite using both helpers.

Thoughts?

[igrigorik]

Instead of checking for "aget", I think you can just check for Synchrony and adjust your logic to use the async methods? Should be cleaner.

[aenain]

Another solution apart from a dedicated helper for event streams is to add with_async_http to goliath/test_helper like below. What do you think about that?

# events_app_spec.rb
require 'json'
require 'redis'
require 'em-eventsource'
require 'goliath/test_helper'

Goliath.env = :test

require_relative '../events_app'

RSpec.describe 'EventsApp' do
  include Goliath::TestHelper

  def with_async_http
    klass = EM::HttpConnection

    begin
      klass.send(:class_eval) do
        alias :sget :get
        alias :get :aget
      end
      yield if block_given?
    ensure
      klass.send(:class_eval) do
        alias :get :sget
        remove_method :sget
      end
    end
  end

  it 'streams events from redis' do
    messages = []
    content = { 'id' => 13 }.to_json

    with_api(EventsApp, {}) do |server|
      source = EM::EventSource.new("http://#{server.address}:#{server.port}/events/applications/1")
      source.on('update') { |msg| messages << msg }
      with_async_http { source.start }

      EM.add_timer(0.1) do
        redis = EM::Hiredis.connect
        redis.publish('app:1:update', content)
      end

      EM.add_timer(0.2) { stop }
    end

    expect(messages).to include(content)
  end
end

[aenain]

#309 resolves the issue.