From 0bc3525b104da074c99c99d95722632764a1c681 Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Wed, 28 May 2014 17:15:10 +1000
Subject: [PATCH] BUGFIX: more robust onebox implementation

---
 lib/oneboxer.rb                  | 33 +++++++++++++++++++-------------
 spec/components/oneboxer_spec.rb | 10 ++++++++++
 2 files changed, 30 insertions(+), 13 deletions(-)
 create mode 100644 spec/components/oneboxer_spec.rb

diff --git a/lib/oneboxer.rb b/lib/oneboxer.rb
index 7930cd4b5a8..8a9a73707c2 100644
--- a/lib/oneboxer.rb
+++ b/lib/oneboxer.rb
@@ -35,12 +35,20 @@ module Oneboxer
     if c = Rails.cache.read(onebox_cache_key(url))
       c[:onebox]
     end
+  rescue => e
+    invalidate(url)
+    Rails.logger.warn("invalid cached onebox for #{url} #{e}")
+    ""
   end
 
   def self.cached_preview(url)
     if c = Rails.cache.read(onebox_cache_key(url))
       c[:preview]
     end
+  rescue => e
+    invalidate(url)
+    Rails.logger.warn("invalid cached preview for #{url} #{e}")
+    ""
   end
 
   def self.oneboxer_exists_for_url?(url)
@@ -110,21 +118,20 @@ module Oneboxer
   end
 
   def self.onebox_raw(url)
-    Rails.cache.fetch(onebox_cache_key(url)){
-      begin
+    Rails.cache.fetch(onebox_cache_key(url), expires_in: 1.day){
+      # This might be able to move to whenever the SiteSetting changes?
+      Oneboxer.add_discourse_whitelists
 
-        # This might be able to move to whenever the SiteSetting changes?
-        Oneboxer.add_discourse_whitelists
-
-        r = Onebox.preview(url, cache: {})
-        {
-          onebox: r.to_s,
-          preview: r.try(:placeholder_html).to_s
-        }
-      rescue => e
-        Discourse.handle_exception(e, url: url)
-      end
+      r = Onebox.preview(url, cache: {})
+      {
+        onebox: r.to_s,
+        preview: r.try(:placeholder_html).to_s
+      }
     }
+  rescue => e
+    Discourse.handle_exception(e, url: url)
+    # return a blank hash, so rest of the code works
+    {preview: "", onebox: ""}
   end
 
 end
diff --git a/spec/components/oneboxer_spec.rb b/spec/components/oneboxer_spec.rb
new file mode 100644
index 00000000000..41e492be048
--- /dev/null
+++ b/spec/components/oneboxer_spec.rb
@@ -0,0 +1,10 @@
+require 'spec_helper'
+require_dependency 'oneboxer'
+
+describe Oneboxer do
+  it "returns blank string for an invalid onebox" do
+    Oneboxer.preview("http://boom.com").should == ""
+    Oneboxer.onebox("http://boom.com").should == ""
+  end
+end
+