Merge pull request mlibrary#9 from mlibrary/feature/job_error

queue item error reporting

Author: malakai97

app/controllers/v1/queue_items_controller.rb

@@ -9,11 +9,7 @@ def index
     def show
       @queue_item = QueueItem.find(params[:id])
       authorize @queue_item
-      if @queue_item.bag
-        head 303, location: v1_bag_url(@queue_item.bag)
-      else
-        render template: "v1/queue_items/show", status: 200
-      end
+      render template: "v1/queue_items/show", status: 200
     end
 
     # POST /v1/requests/:bag_id/complete

app/jobs/bag_move_job.rb

@@ -1,21 +1,31 @@
 class BagMoveJob < ApplicationJob
   def perform(queue_item, src_path, dest_path)
-    File.rename(src_path,dest_path)
 
-    # When a bag is just a completed request 
-    #  - start a transaction that updates the request to complete
-    #  - move the bag into place
-    #  - success: commit the transaction
-    #  - failure (exception) - transaction automatically rolls back
-    queue_item.transaction do
-      binding.pry
-      queue_item.bag = bag_type(queue_item).create!(
-        bag_id: queue_item.request.bag_id,
-        user: queue_item.user,
-        storage_location: dest_path,
-        external_id: queue_item.request.external_id
-      )
+    begin
+      File.rename(src_path,dest_path)
+
+      # When a bag is just a completed request 
+      #  - start a transaction that updates the request to complete
+      #  - move the bag into place
+      #  - success: commit the transaction
+      #  - failure (exception) - transaction automatically rolls back
+      queue_item.transaction do
+        queue_item.bag = bag_type(queue_item).create!(
+          bag_id: queue_item.request.bag_id,
+          user: queue_item.user,
+          storage_location: dest_path,
+          external_id: queue_item.request.external_id,
+        )
+        queue_item.status = :done
+        queue_item.save!
+      end
+    rescue => exception
+      # attempt to log
+      queue_item.error = exception.to_s
+      queue_item.status = :failed
       queue_item.save!
+      # re-raise
+      raise exception
     end
   end
 

lib/uploader.rb

@@ -17,8 +17,7 @@ def upload
     req = make_request
     rsync_bag(req["upload_link"])
     qitem = complete_request(req)
-    bag = wait_for_bag(qitem)
-    p bag
+    print_result(wait_for_bag(qitem))
   end
 
   def bag_id
@@ -29,6 +28,14 @@ def bag_id
 
   attr_accessor :request_params, :api_key, :bag_path
 
+  def print_result(qitem_result)
+    if qitem_result["status"] == "DONE"
+      pp chipmunk_get(qitem_result["bag"])
+    else
+      pp qitem_result
+    end
+  end
+
   def require_chipmunk_bag_tags(tags)
   ["External-Identifier", 
    "Bag-ID", 
@@ -70,18 +77,14 @@ def complete_request(request)
   end
 
   def wait_for_bag(qitem)
+    result = qitem
     loop do
       # update qitem
-      result = chipmunk_get("/v1/queue/#{qitem["id"]}")
-      break if qitem["status"] != "PENDING"
+      return result if result["status"] != "PENDING"
       puts "Waiting for queue item to be processed"
       sleep 10
+      result = chipmunk_get("/v1/queue/#{qitem["id"]}")
     end
-
-    # TODO: error h&&ling 
-    # get 200 or 303
-
-    # get /bags/:bag_id, display
   end
 
   def chipmunk_post(endpoint,params = {})
@@ -104,9 +107,15 @@ def follow_redir_and_parse(response)
   end
 
   def chipmunk_request(method,endpoint,**kwargs)
+    begin
     follow_redir_and_parse(RestClient::Request.execute(method: method, 
                                 url: CHIPMUNK_URL + endpoint,
                                 headers: auth_header,
                                 **kwargs))
+    rescue RestClient::InternalServerError => e
+      puts e.to_s
+      puts JSON.parse(e.response)["exception"]
+      exit 1
+    end
   end
 end

spec/controllers/v1/queue_items_controller_spec.rb

@@ -54,79 +54,10 @@
       end
 
       context "queue_item has a bag (is complete)" do
-        before(:each) do
-          request.headers.merge! auth_header
-          get :show, params: {id: record.id}
-        end
-
-        context "as unauthenticated user" do
-          include_context "as unauthenticated user"
-          let(:record) { done_proc.call }
-          it "returns 401" do
-            expect(response).to have_http_status(401)
-          end
-          it "renders nothing" do
-            expect(response).to render_template(nil)
-          end
-        end
-
-        context "as underprivileged user" do
-          include_context "as underprivileged user"
-
-          context "the record belongs to the user" do
-            let(:record) { done_proc.call(user) }
-            it "returns 303" do
-              expect(response).to have_http_status(303)
-            end
-            it "correctly sets the location header" do
-              expect(response.location).to eql(v1_bag_url(record.bag))
-            end
-            it "renders nothing" do
-              expect(response).to render_template(nil)
-            end
-          end
-
-          context "the record does not belong to the user" do
-            let(:record) { done_proc.call }
-            it "returns 403" do
-              expect(response).to have_http_status(403)
-            end
-            it "renders nothing" do
-              expect(response).to render_template(nil)
-            end
-          end
-
-        end
-
-        context "as admin" do
-          include_context "as admin user"
-
-          context "the record belongs to the user" do
-            let(:record) { done_proc.call(user) }
-            it "returns 303" do
-              expect(response).to have_http_status(303)
-            end
-            it "correctly sets the location header" do
-              expect(response.location).to eql(v1_bag_url(record.bag))
-            end
-            it "renders nothing" do
-              expect(response).to render_template(nil)
-            end
-          end
-
-          context "the record does not belong to the user" do
-            let(:record) { done_proc.call }
-            it "returns 303" do
-              expect(response).to have_http_status(303)
-            end
-            it "correctly sets the location header" do
-              expect(response.location).to eql(v1_bag_url(record.bag))
-            end
-            it "renders nothing" do
-              expect(response).to render_template(nil)
-            end
-          end
-
+        it_behaves_like "a show endpoint" do
+          let(:key) { :id }
+          let(:factory) { done_proc }
+          let(:assignee) { :queue_item }
         end
       end
 

spec/jobs/bag_move_job_spec.rb

@@ -0,0 +1,61 @@
+require 'rails_helper'
+
+RSpec.describe BagMoveJob do
+  let (:queue_item) { Fabricate(:queue_item) }
+  let (:srcpath) { 'foo' }
+  let (:destpath) { 'bar' }
+
+  class InjectedError < RuntimeError
+  end
+
+  describe '#perform' do
+    subject { described_class.perform_now(queue_item,srcpath,destpath) }
+
+    context "when the move succeeds" do
+      before(:each) do
+        allow(File).to receive(:rename).with(srcpath,destpath).and_return true
+      end
+
+      it "moves the bag" do
+        expect(File).to receive(:rename).with(srcpath,destpath)
+        subject
+      end
+
+      it "on success, updates the queue_item to status :done" do
+        subject
+        expect(queue_item.status).to eql("done")
+      end
+    end
+
+    context "when there is an error" do
+      before(:each) do
+        allow(File).to receive(:rename).with(srcpath,destpath).and_raise InjectedError, 'injected error'
+      end
+
+      it "re-raises the exception" do
+        expect { subject }.to raise_exception(InjectedError)
+      end
+
+      it "updates the queue_item to status 'failed'" do
+        begin
+          subject
+        rescue InjectedError
+        end
+
+        expect(queue_item.status).to eql("failed")
+      end
+
+      it "records the error in the queue_item" do
+
+        begin
+          subject
+        rescue InjectedError
+        end
+
+        expect(queue_item.error).to match(/injected error/)
+      end
+    end
+
+  end
+end
+