BUGFIX: more robust onebox implementation

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

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
+