PERF: hijack onebox requests so they do not use up a unicorn worker

app/controllers/application_controller.rb

@@ -11,12 +11,14 @@ require_dependency 'distributed_cache'
 require_dependency 'global_path'
 require_dependency 'secure_session'
 require_dependency 'topic_query'
+require_dependency 'hijack'
 
 class ApplicationController < ActionController::Base
   include CurrentUser
   include CanonicalURL::ControllerExtensions
   include JsonError
   include GlobalPath
+  include Hijack
 
   attr_reader :theme_key
 

app/controllers/onebox_controller.rb

@@ -4,7 +4,6 @@ class OneboxController < ApplicationController
   before_action :ensure_logged_in
 
   def show
-
     unless params[:refresh] == 'true'
       preview = Oneboxer.cached_preview(params[:url])
       preview.strip! if preview.present?
@@ -14,19 +13,23 @@ class OneboxController < ApplicationController
     # only 1 outgoing preview per user
     return render(body: nil, status: 429) if Oneboxer.is_previewing?(current_user.id)
 
-    Oneboxer.preview_onebox!(current_user.id)
+    user_id = current_user.id
+    invalidate = params[:refresh] == 'true'
+    url = params[:url]
 
-    preview = Oneboxer.preview(params[:url], invalidate_oneboxes: params[:refresh] == 'true')
-    preview.strip! if preview.present?
+    hijack do
+      Oneboxer.preview_onebox!(user_id)
 
-    Scheduler::Defer.later("Onebox previewed") {
-      Oneboxer.onebox_previewed!(current_user.id)
-    }
+      preview = Oneboxer.preview(url, invalidate_oneboxes: invalidate)
+      preview.strip! if preview.present?
 
-    if preview.blank?
-      render body: nil, status: 404
-    else
-      render plain: preview
+      Oneboxer.onebox_previewed!(user_id)
+
+      if preview.blank?
+        render body: nil, status: 404
+      else
+        render plain: preview
+      end
     end
   end
 

lib/hijack.rb

@@ -0,0 +1,69 @@
+# frozen_string_literal: true
+
+# This module allows us to hijack a request and send it to the client in the deferred job queue
+# For cases where we are making remote calls like onebox or proxying files and so on this helps
+# free up a unicorn worker while the remote IO is happening
+module Hijack
+  class Binder
+    attr_reader :content_type, :body, :status
+
+    def initialize
+      @content_type = 'text/plain'
+      @status = 500
+      @body = ""
+    end
+
+    def render(opts)
+      if opts[:status]
+        @status = opts[:status].to_i
+      else
+        @status = 200
+      end
+
+      if opts.key?(:body)
+        @body = opts[:body].to_s
+      end
+
+      if opts.key?(:plain)
+        @content_type = 'text/plain'
+        @body = opts[:plain].to_s
+      end
+    end
+  end
+
+  def hijack(&blk)
+    if hijack = request.env['rack.hijack']
+      io = hijack.call
+
+      Scheduler::Defer.later("hijack work") do
+
+        begin
+          # do this first to confirm we have a working connection
+          # before doing any work
+          io.write "HTTP/1.1 "
+
+          binder = Binder.new
+          begin
+            binder.instance_eval(&blk)
+          rescue => e
+            Rails.logger.warn("Failed to process hijacked response correctly #{e}")
+          end
+
+          io.write "#{binder.status} OK\r\n"
+          io.write "Content-Length: #{binder.body.bytesize}\r\n"
+          io.write "Content-Type: #{binder.content_type}\r\n"
+          io.write "Connection: close\r\n"
+          io.write "\r\n"
+          io.write binder.body
+          io.close
+        rescue Errno::EPIPE, IOError
+          # happens if client terminated before we responded, ignore
+        end
+      end
+      # not leaked out, we use 418 ... I am a teapot to denote that we are hijacked
+      render plain: "", status: 418
+    else
+      blk.call
+    end
+  end
+end

spec/components/hijack_spec.rb

@@ -0,0 +1,70 @@
+require 'rails_helper'
+
+describe Hijack do
+  class Hijack::Tester
+    attr_reader :io
+
+    include Hijack
+    def initialize
+      @io = StringIO.new
+    end
+
+    def hijack_test(&blk)
+      hijack do
+        self.instance_eval(&blk)
+      end
+    end
+
+    def request
+      @req ||= ActionController::TestRequest.new(
+        { "rack.hijack" => lambda { @io } },
+        nil,
+        nil
+      )
+    end
+
+    def render(*opts)
+      # don't care
+    end
+  end
+
+  let :tester do
+    Hijack::Tester.new
+  end
+
+  it "renders non 200 status if asked for" do
+    tester.hijack_test do
+      render body: "hello world", status: 402
+    end
+
+    expect(tester.io.string).to include("402")
+    expect(tester.io.string).to include("world")
+  end
+
+  it "renders stuff correctly if it works" do
+    tester.hijack_test do
+      render plain: "hello world"
+    end
+
+    result = "HTTP/1.1 200 OK\r\nContent-Length: 11\r\nContent-Type: text/plain\r\nConnection: close\r\n\r\nhello world"
+    expect(tester.io.string).to eq(result)
+  end
+
+  it "returns 500 by default" do
+    tester.hijack_test
+
+    expected = "HTTP/1.1 500 OK\r\nContent-Length: 0\r\nContent-Type: text/plain\r\nConnection: close\r\n\r\n"
+    expect(tester.io.string).to eq(expected)
+  end
+
+  it "does not run the block if io is closed" do
+    tester.io.close
+
+    ran = false
+    tester.hijack_test do
+      ran = true
+    end
+
+    expect(ran).to eq(false)
+  end
+end

spec/controllers/onebox_controller_spec.rb

@@ -21,17 +21,37 @@ describe OneboxController do
 
     describe "cached onebox" do
 
-      let(:body) { "This is a cached onebox body" }
-
-      before do
-        Oneboxer.expects(:cached_preview).with(url).returns(body)
-        Oneboxer.expects(:preview).never
-        get :show, params: { url: url, user_id: @user.id }, format: :json
-      end
-
       it "returns the cached onebox response in the body" do
+        onebox_html = <<~HTML
+          <html>
+          <head>
+            <meta property="og:title" content="Fred the title">
+            <meta property="og:description" content="this is bodycontent">
+          </head>
+          <body>
+             <p>body</p>
+          </body>
+          <html>
+        HTML
+
+        url = "http://noodle.com/"
+
+        stub_request(:head, url).
+          to_return(status: 200, body: "", headers: {}).then.to_raise
+
+        stub_request(:get, url)
+          .to_return(status: 200, headers: {}, body: onebox_html).then.to_raise
+
+        get :show, params: { url: url, user_id: @user.id, refresh: "true" }, format: :json
+
         expect(response).to be_success
-        expect(response.body).to eq(body)
+        expect(response.body).to include('Fred')
+        expect(response.body).to include('bodycontent')
+
+        get :show, params: { url: url, user_id: @user.id }, format: :json
+        expect(response).to be_success
+        expect(response.body).to include('Fred')
+        expect(response.body).to include('bodycontent')
       end
 
     end