* better error reporting for bad connections on join session

ruby/lib/jam_ruby.rb

@@ -28,6 +28,7 @@ require "jam_ruby/constants/validation_messages"
 require "jam_ruby/errors/jam_permission_error"
 require "jam_ruby/errors/state_error"
 require "jam_ruby/errors/jam_argument_error"
+require "jam_ruby/errors/jam_record_not_found"
 require "jam_ruby/errors/conflict_error"
 require "jam_ruby/lib/app_config"
 require "jam_ruby/lib/s3_manager_mixin"

ruby/lib/jam_ruby/connection_manager.rb

@@ -408,7 +408,15 @@ SQL
       ConnectionManager.active_record_transaction do |connection_manager|
         db_conn = connection_manager.pg_conn
 
-        connection = Connection.find_by_client_id_and_user_id!(client_id, user.id)
+        connection = Connection.find_by_client_id(client_id)
+
+        if connection.nil?
+          raise JamRecordNotFound.new("Unable to find connection by client_id #{client_id}", 'Connection')
+        elsif connection.user_id.nil?
+          raise JamPermissionError, "no user_id associated with connection #{client_id}"
+        elsif connection.user_id != user.id
+          raise JamPermissionError, "wrong user_id associated with connection #{client_id}"
+        end
 
         connection.join_the_session(music_session, as_musician, tracks, user, audio_latency, video_sources)
         JamRuby::MusicSessionUserHistory.join_music_session(user.id, music_session.id)

ruby/lib/jam_ruby/errors/jam_record_not_found.rb

@@ -0,0 +1,12 @@
+module JamRuby
+  class JamRecordNotFound < StandardError
+
+    attr_accessor :missing_message, :record_type
+
+    def initialize(message, record_type)
+      @message = message
+      @missing_message = message
+      @record_type = record_type
+    end
+  end
+end

ruby/spec/jam_ruby/connection_manager_spec.rb

@@ -107,7 +107,7 @@ describe ConnectionManager, no_transaction: true do
     cc.locidispid.should == 17192000002
     cc.udp_reachable.should == true
 
-    @connman.reconnect(cc, channel_id, nil, "33.1.2.3", STALE_TIME, EXPIRE_TIME, false, GATEWAY, false)
+    @connman.reconnect(cc, channel_id, nil, "33.1.2.3", STALE_TIME, EXPIRE_TIME, false, GATEWAY)
 
     cc = Connection.find_by_client_id!(client_id)
     cc.connected?.should be_true
@@ -147,7 +147,7 @@ describe ConnectionManager, no_transaction: true do
     cc.locidispid.should == 17192000002
     cc.udp_reachable.should == false
 
-    @connman.reconnect(cc, channel_id, nil, "33.1.2.3", STALE_TIME, EXPIRE_TIME, nil, GATEWAY, false) # heartbeat passes nil in for udp_reachable
+    @connman.reconnect(cc, channel_id, nil, "33.1.2.3", STALE_TIME, EXPIRE_TIME, nil, GATEWAY) # heartbeat passes nil in for udp_reachable
 
     cc = Connection.find_by_client_id!(client_id)
     cc.connected?.should be_true
@@ -356,7 +356,7 @@ describe ConnectionManager, no_transaction: true do
 
     user = User.find(user_id)
 
-    expect { @connman.join_music_session(user, client_id, music_session, true, TRACKS, 10) }.to raise_error(ActiveRecord::RecordNotFound)
+    expect { @connman.join_music_session(user, client_id, music_session, true, TRACKS, 10) }.to raise_error(JamRuby::JamRecordNotFound)
 
   end
 
@@ -429,7 +429,7 @@ describe ConnectionManager, no_transaction: true do
 
     @connman.create_connection(user_id, client_id, channel_id, "1.1.1.1", 'client', STALE_TIME, EXPIRE_TIME, REACHABLE, GATEWAY, false)
     # specify real user id, but not associated with this session
-    expect { @connman.join_music_session(user, client_id, music_session, true, TRACKS, 10) } .to raise_error(ActiveRecord::RecordNotFound)
+    expect { @connman.join_music_session(user, client_id, music_session, true, TRACKS, 10) } .to raise_error(JamRuby::JamPermissionError)
   end
 
   it "join_music_session fails if no music_session" do
@@ -455,7 +455,7 @@ describe ConnectionManager, no_transaction: true do
 
     @connman.create_connection(user_id, client_id, channel_id, "1.1.1.1", 'client', STALE_TIME, EXPIRE_TIME, REACHABLE, GATEWAY, false)
     # specify real user id, but not associated with this session
-    expect { @connman.join_music_session(user, client_id, music_session, true, TRACKS, 10) } .to raise_error(ActiveRecord::RecordNotFound)
+    expect { @connman.join_music_session(user, client_id, music_session, true, TRACKS, 10) } .to raise_error(JamRuby::JamPermissionError)
   end
 
 

ruby/spec/jam_ruby/models/active_music_session_spec.rb

@@ -17,7 +17,14 @@ describe ActiveMusicSession do
     it "fails gracefully when no connection" do
       music_session = FactoryGirl.create(:active_music_session, :creator => user, :musician_access => false)
 
-      expect { ActiveMusicSession.participant_create(user, music_session.id, "junk", true,  nil, 5) }.to raise_error(ActiveRecord::RecordNotFound)
+      begin
+        ActiveMusicSession.participant_create(user, music_session.id, "junk", true,  nil, 5)
+        false.should be_true
+      rescue JamRuby::JamRecordNotFound => e
+        e.record_type.should eql "Connection"
+        e.missing_message.should eql "Unable to find connection by client_id junk"
+      end
+
     end
 
     it "succeeds no active music session" do
@@ -34,12 +41,6 @@ describe ActiveMusicSession do
       expect { ActiveMusicSession.participant_create(user, 'bad music session ID', conn.client_id, true,  nil, 5) }.to raise_error(ActiveRecord::RecordNotFound)
     end
 
-    it "fails gracefully when invalid music session Id" do
-      music_session = FactoryGirl.create(:active_music_session, :creator => user, :musician_access => false)
-      conn = FactoryGirl.create(:connection, :user => user)
-      ActiveMusicSession.participant_create(user, 'bad music session ID', conn.client_id, true,  nil, 5)
-    end
-
     it "pulls out of other session" do
       music_session1 = FactoryGirl.create(:active_music_session, :creator => user, :musician_access => false)
       music_session2 = FactoryGirl.create(:active_music_session, :creator => user, :musician_access => false)
@@ -52,7 +53,8 @@ describe ActiveMusicSession do
     it "user-less connection bails appropriately" do
       music_session1 = FactoryGirl.create(:active_music_session, :creator => user, :musician_access => false)
       conn = FactoryGirl.create(:connection)
-      expect {  ActiveMusicSession.participant_create(user, music_session1.id, conn.client_id, true,  nil, 5) }.to raise_error(ActiveRecord::RecordNotFound)
+      ActiveRecord::Base.connection.execute("update connections set user_id = NULL where connections.id = '#{conn.id}'")
+      expect {  ActiveMusicSession.participant_create(user, music_session1.id, conn.client_id, true,  nil, 5) }.to raise_error(JamRuby::JamPermissionError)
     end
   end
 

web/app/controllers/api_controller.rb

@@ -19,6 +19,10 @@ class ApiController < ApplicationController
     @exception = exception
     render "errors/permission_error", :status => 403
   end
+  rescue_from 'JamRuby::JamRecordNotFound' do |exception|
+    @exception = exception
+    render "errors/record_not_found", :status => 404
+  end
   rescue_from 'JamRuby::ConflictError' do |exception|
     @exception = exception
     render "errors/conflict_error", :status => 409

web/app/views/errors/record_not_found.rabl

@@ -0,0 +1,11 @@
+object @exception
+
+attributes :record_type
+
+node "message" do |e|
+  e.missing_message
+end
+
+node "type" do
+  "JamRecordNotFound"
+end