Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Streaming API does not stream in v1.3.2 #473

Closed
bawNg opened this Issue · 12 comments

3 participants

@bawNg

Issue

Response body is only sent to client when output.close is called (no streaming) when using Sinatra 1.3.2 with Thin.

Test Case

require 'thin'
require 'sinatra/base'

class TestCase < Sinatra::Base
  set :root, File.dirname(__FILE__)

  get '/' do
    stream(:keep_open) do |output|
      output << "Streaming live data...\n"
      timer = EM.add_periodic_timer 0.5 do
        print '.'
        output << '.'
      end
      EM.add_timer 6 do
        puts 'timed out!'
        output << 'timed out!'
        output.close
        timer.cancel
      end
    end
  end
end

TestCase.run! port: 18000
@rkh
Owner

That is because most HTTP servers and clients don't consider a chunk of data to be complete until they receive either a newline (if they consider the data to be text) or a sufficiently large chunk of data for binary files, meaning this is a client side problem. There is not much we can do about this.

@bawNg

Then why does 1.3.0 and 1.3.1 work as expected?

@rkh
Owner

Uh, oh, that is strange. Will investigate.

@carlhoerberg

stream(:keep_open) seems to work when doing a "plain" sinatra app, but not when sub classed..

@rkh
Owner
rkh commented

OK, I'll write an integration test for this running it through Thin/TCP. Wrote the foundation for this last night.

@rkh rkh referenced this issue from a commit
@rkh rkh add test for #473 202f6b2
@rkh
Owner
rkh commented

The added test actually proves that the issue only exists in modular apps. I have no idea why.

@carlhoerberg
@rkh
Owner
rkh commented

Did you do run Sinatra::Application or run Sinatra::Application.new?

@carlhoerberg
@rkh
Owner

Cannot reproduce, your example works for me both with the current master and 1.3.2. Could you check if the tests pass for you, locally?

@rkh rkh closed this
@rkh rkh referenced this issue from a commit
@rkh rkh add test for #473 6c6fa81
@carlhoerberg

TestCase.run! port: 18000 works, but if you use a config.ru file:

require './test_case'
#\ -p 18000
run TestCase

and start rackup it doesn't work for me

@rkh rkh reopened this
@rkh
Owner

OK, I'll check it out.

@rkh rkh closed this in 298ea5b
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.