Cleanup spitball errors

bin/spitball-server

@@ -5,6 +5,9 @@ require 'optparse'
 require 'sinatra'
 require 'json'
 
+# TODO: Sinatra needs to be updated. Once we update let's add in auto-reload.
+# require "sinatra/reloader" if development?
+
 # cargo culting sinatra's option parser, since it has trouble
 # with rubygem stub files
 
@@ -19,10 +22,20 @@ OptionParser.new { |op|
 # always run
 set :run, true
 
+set :root, File.dirname(__FILE__)
+set :views, File.expand_path(File.join(settings.root, '..', 'lib', 'templates'))
+
 mime_type :gemfile, 'text/plain'
 mime_type :lock, 'text/plain'
 mime_type :tgz, 'application/x-compressed'
 
+get '/' do
+ @digests = Spitball::Repo.cached_digests
+ @env = `#{$:.inspect}\n#{Spitball.gem_cmd} env`
+ @protocol_version = Spitball::PROTOCOL_VERSION
+ erb :index
+end
+
 # return json array of cached SHAs
 get '/list' do
  content_type :json
@@ -42,59 +55,110 @@ get '/:digest.:format' do |digest, format|
  send_file Spitball::Repo.bundle_path(digest, format), :type => format
 end
 
+# A common set of validation for the create spitball methods
+before '/create*' do
+ request_version = request.env["HTTP_#{Spitball::PROTOCOL_HEADER.gsub(/-/, '_').upcase}"] || params['protocol_version']
+
+ if request_version != Spitball::PROTOCOL_VERSION
+ halt 403, "Received version #{request_version} but need version #{Spitball::PROTOCOL_VERSION}"
+ end
+
+ if params[:gemfile].nil?
+ halt 400, "gemfile parameter was null. Please supply a valid Gemfile"
+ end
+
+ if params[:gemfile_lock].nil?
+ halt 400, "gemfile_lock parameter was null. Please supply a valid Gemfile.lock"
+ end
+
+ @without = request_version = request.env["HTTP_#{Spitball::WITHOUT_HEADER.gsub(/-/, '_').upcase}"] || params['without']
+ @without &&= @without.split(',')
+end
+
 class Streamer
  def initialize(io); @io = io end
  def each; while buf = @io.read(200); yield buf end end
 end
 
-# POST a gemfile. Returns 201 Created if the bundle already exists, or
+post '/create_by_form' do
+
+ # More specific validation checking for form upload data
+ if params[:gemfile][:tempfile].nil?
+ halt 400, "gemfile parameter was null. Please supply a valid Gemfile"
+ end
+
+ if params[:gemfile_lock][:tempfile].nil?
+ halt 400, "gemfile_lock parameter was null. Please supply a valid Gemfile.lock"
+ end
+
+ # Since this is just a debugging form, I think it's ok to let any exception bubble up to the user
+ ball = Spitball.new(params[:gemfile][:tempfile].read, params[:gemfile_lock][:tempfile].read, :without => @without)
+
+ spitball_url = "/#{ball.digest}.tgz"
+
+ if ball.cached?
+ redirect to(spitball_url)
+ else
+ status 202
+ create_spitball(ball)
+ body "Your spitball is being generated. Please check #{spitball_url} in a few minutes"
+ end
+end
+
+# POST a gemfile from a command line client. Returns 201 Created if the bundle already exists, or
 # 202 Accepted if it does not. The body of the response is the URI for
 # the tarball.
 post '/create' do
- request_version = request.env["HTTP_#{Spitball::PROTOCOL_HEADER.gsub(/-/, '_').upcase}"]
- if request_version != Spitball::PROTOCOL_VERSION
- status 403
- "Received version #{request_version} but need version #{Spitball::PROTOCOL_VERSION}"
+
+ # Spitball was previously returning the entire html stack trace on errors. This made it extremely difficult
+ # to debug on the client side. Gracefully handle errors here and return them nicely to the user.
+ begin
+ ball = Spitball.new(params[:gemfile], params[:gemfile_lock], :without => @without)
+ rescue Object => e
+ puts e.backtrace
+ halt 500, e.message
+ end
+
+ response['Location'] = "#{request.url.split("/create").first}/#{ball.digest}.tgz"
+
+ if ball.cached?
+ status 201
+ "Bundle tarball pre-cached.\n"
  else
- without = request_version = request.env["HTTP_#{Spitball::WITHOUT_HEADER.gsub(/-/, '_').upcase}"]
- without &&= without.split(',')
- ball = Spitball.new(params['gemfile'], params['gemfile_lock'], :without => without)
- url = "#{request.url.split("/create").first}/#{ball.digest}.tgz"
- response['Location'] = url
-
- if ball.cached?
- status 201
- "Bundle tarball pre-cached.\n"
- else
- status 202
-
- i,o = IO.pipe
- o.sync = true
-
- # fork twice. once so we can have a pid to detach from in order to clean up,
- # twice in order to properly redirect stdout for underlying shell commands.
- pid = fork do
- baller = open("|-")
- if baller == nil # child
- $stdout.sync = true
- begin
- ball.cache!
- rescue Object => e
- puts "#{e.class}: #{e}", e.backtrace.map{|l| "\t#{l}" }
- end
- else
- while buf = baller.read(200)
- $stdout.print buf
- o.print buf
- end
- end
- exit!
- end
+ status 202
+ create_spitball(ball)
+ end
+end
+
+private
+
+def create_spitball(spitball)
 
-  o.close
-  Process.detach(pid)
+ i,o = IO.pipe
+ o.sync = true
 
- Streamer.new(i)
+ # fork twice. once so we can have a pid to detach from in order to clean up,
+ # twice in order to properly redirect stdout for underlying shell commands.
+ pid = fork do
+ baller = open("|-")
+ if baller == nil # child
+ $stdout.sync = true
+ begin
+ spitball.cache!
+ rescue Object => e
+ puts "#{e.class}: #{e}", e.backtrace.map{|l| "\t#{l}" }
+ end
+ else
+ while buf = baller.read(200)
+ $stdout.print buf
+ o.print buf
+ end
  end
+ exit!
  end
+
+ o.close
+ Process.detach(pid)
+
+ Streamer.new(i)
 end

lib/spitball.rb

@@ -17,6 +17,7 @@ class Spitball
  class ServerFailure < StandardError; end
  class ClientError < StandardError; end
  class BundleCreationFailure < StandardError; end
+ class GemfileParsingError < StandardError; end
 
  def self.gem_cmd
  ENV['GEM_CMD'] || 'gem'
@@ -37,7 +38,19 @@ def initialize(gemfile, gemfile_lock, options = {})
  @gemfile_lock = gemfile_lock
  @options = options
  @without = options[:without].is_a?(Enumerable) ? options[:without].map(&:to_sym) : (options[:without] ? [options[:without].to_sym] : [])
- @parsed_lockfile, @dsl = Bundler::FakeLockfileParser.new(gemfile_lock), Bundler::FakeDsl.new(gemfile)
+
+ begin
+ @parsed_lockfile = Bundler::FakeLockfileParser.new(gemfile_lock)
+ rescue StandardError => se
+ raise GemfileParsingError, "There was an error parsing your gemfile.lock. Please ensure the file is valid and try again"
+ end
+
+ begin
+ @dsl = Bundler::FakeDsl.new(gemfile)
+ rescue StandardError => se
+ raise GemfileParsingError, "There was an error parsing your Gemfile. Please ensure the file is valid and try again"
+ end
+
  raise "You need to run bundle install before you can use spitball" unless (@parsed_lockfile.dependencies.map{|d| d.name}.uniq.sort == @dsl.__gem_names.uniq.sort)
  @groups_to_install = @dsl.__groups.keys - @without
  end

lib/spitball/version.rb

@@ -1,3 +1,3 @@
 class Spitball
- VERSION = '0.7.3' unless const_defined?(:VERSION)
+ VERSION = '0.8.0' unless const_defined?(:VERSION)
 end

lib/templates/index.erb

@@ -0,0 +1,27 @@
+<h2>Spitball!</h2>
+
+<div style="float: left; width: 50%;">
+ <h3>Env</h3>
+ <div><pre><%= @env %></pre></div>
+ <h3><%= @digests.length %> Cached Digests</h3>
+ <div style="min-height: 400px; overflow: auto;">
+ <ul>
+ <% for digest in @digests %>
+ <li><%= digest %> <a href="/<%= digest %>.tgz">tgz</a></li>
+ <% end %>
+ </ul>
+</div>
+</div>
+
+<div style="float: left;">
+ <h3>Upload</h3>
+ <form method="post" action="/create_by_form" enctype='multipart/form-data'>
+ <div>Gemfile: <input type="file" name="gemfile"/></div>
+ <div>Gemfile.lock: <input type="file" name="gemfile_lock"/></div>
+ <div>Without groups: <input type="text" name="without"/></div>
+ <input type="hidden" name="protocol_version" value="<%= @protocol_version %>"/>
+ <input type="submit" value="submit"/>
+ </form>
+</div>
+
+<div style="clear: both;"></div>

spec/spitball_spec.rb

@@ -2,8 +2,6 @@
 
 describe Spitball do
  before do
- use_success_bundler
-
  @gemfile = <<-end_gemfile
  source :rubygems
  gem "json_pure"
@@ -300,10 +298,89 @@
 
  describe "create_bundle failure" do
  it "should raise on create_bundle" do
+ use_success_bundler
  proc { Spitball.new(@gemfile, @lockfile) }.should raise_error
  end
  end
  end
+
+ context "error handling" do
+
+ it "should throw an exception on an invalid Gemfile" do
+
+ gemfile = <<-end_gemfile
+ source :rubygems
+ gem "activerecord"
+ raise 'foo'
+ end_gemfile
+
+ lockfile = <<-end_lockfile.strip.gsub(/\n[ ]{8}/m, "\n")
+ GEM
+ remote: http://rubygems.org/
+ specs:
+ activemodel (3.0.1)
+ activesupport (= 3.0.1)
+ builder (~> 2.1.2)
+ i18n (~> 0.4.1)
+ activerecord (3.0.1)
+ activemodel (= 3.0.1)
+ activesupport (= 3.0.1)
+ arel (~> 1.0.0)
+ tzinfo (~> 0.3.23)
+ activesupport (3.0.1)
+ arel (1.0.1)
+ activesupport (~> 3.0.0)
+ builder (2.1.2)
+ i18n (0.4.2)
+ tzinfo (0.3.23)
+
+ PLATFORMS
+ ruby
+
+ DEPENDENCIES
+ activerecord
+ end_lockfile
+
+ proc { Spitball.new(gemfile, lockfile) }.should raise_error(Spitball::GemfileParsingError, 'There was an error parsing your Gemfile. Please ensure the file is valid and try again')
+ end
+
+ it "should throw an exception on an invalid Gemfile.lock" do
+
+ gemfile = <<-end_gemfile
+ source :rubygems
+ gem "activerecord"
+ end_gemfile
+
+ lockfile = <<-end_lockfile.strip.gsub(/\n[ ]{8}/m, "\n")
+ SYNTAXERROR
+ remote: http://rubygems.org/
+ specs:
+ activemodel (3.0.1)
+ activesupport (= 3.0.1)
+ builder (~> 2.1.2)
+ i18n (~> 0.4.1)
+ activerecord (3.0.1)
+ activemodel (= 3.0.1)
+ activesupport (= 3.0.1)
+ arel (~> 1.0.0)
+ tzinfo (~> 0.3.23)
+ activesupport (3.0.1)
+ arel (1.0.1)
+ activesupport (~> 3.0.0)
+ builder (2.1.2)
+ i18n (0.4.2)
+ tzinfo (0.3.23)
+
+ PLATFORMS
+ ruby
+
+ DEPENDENCIES
+ activerecord
+ end_lockfile
+
+ proc { Spitball.new(gemfile, lockfile) }.should raise_error(Spitball::GemfileParsingError, 'There was an error parsing your gemfile.lock. Please ensure the file is valid and try again')
+ end
+ end
 end
 
 describe Bundler::FakeDsl do