Changing how the sphinxapi handles socket errors.

apps/search/clients.py

@@ -1,3 +1,6 @@
+import logging
+import socket
+
 from django.conf import settings
 
 from .sphinxapi import SphinxClient
@@ -33,6 +36,12 @@
     (r'\s+',),
 )
 
+log = logging.getLogger('k.search')
+
+
+class SearchError(Exception):
+    pass
+
 
 class SearchClient(object):
     """
@@ -58,7 +67,44 @@ def __init__(self):
 
                 self.compiled_patterns.append(p)
 
-    def query(self, query, filters): abstract
+    def query(self, query, filters=None):
+        """
+        Query the search index.
+        """
+
+        if filters is None:
+            filters = []
+
+        sc = self.sphinx
+        sc.ResetFilters()
+
+        sc.SetFieldWeights(self.weights)
+
+        for f in filters:
+            if f.get('range', False):
+                sc.SetFilterRange(f['filter'], f['min'],
+                                  f['max'], f.get('exclude', False))
+            else:
+                sc.SetFilter(f['filter'], f['value'],
+                             f.get('exclude', False))
+
+
+        try:
+            result = sc.Query(query, self.index)
+        except socket.timeout:
+            log.error("Query has timed out!")
+            raise SearchError("Query has timed out!")
+        except socket.error, msg:
+            log.error("Query socket error: %s" % msg)
+            raise SearchError("Could not execute your search!")
+        except Exception, e:
+            log.error("Sphinx threw an unknown exception: %s" % e)
+            raise SearchError("Sphinx threw an unknown exception!")
+
+        if result:
+            return result['matches']
+        else:
+            return []
 
     def excerpt(self, result, query):
         """
@@ -97,67 +143,12 @@ class ForumClient(SearchClient):
     Search the forum
     """
     index = 'forum_threads'
-
-    def query(self, query, filters=None):
-        """
-        Search through forum threads
-        """
-
-        if filters is None:
-            filters = []
-
-        sc = self.sphinx
-        sc.ResetFilters()
-
-        sc.SetFieldWeights({'title': 4, 'content': 3})
-
-        for f in filters:
-            if f.get('range', False):
-                sc.SetFilterRange(f['filter'], f['min'],
-                                  f['max'], f.get('exclude', False))
-            else:
-                sc.SetFilter(f['filter'], f['value'],
-                             f.get('exclude', False))
-
-
-        result = sc.Query(query, 'forum_threads')
-
-        if result:
-            return result['matches']
-        else:
-            return []
+    weights = {'title': 4, 'content': 3}
 
 
 class WikiClient(SearchClient):
     """
     Search the knowledge base
     """
     index = 'wiki_pages'
-
-    def query(self, query, filters=None):
-        """
-        Search through the wiki (ie KB)
-        """
-
-        if filters is None:
-            filters = []
-
-        sc = self.sphinx
-        sc.ResetFilters()
-
-        sc.SetFieldWeights({'title': 4, 'keywords': 3})
-
-        for f in filters:
-            if f.get('range', False):
-                sc.SetFilterRange(f['filter'], f['min'],
-                                  f['max'], f.get('exclude', False))
-            else:
-                sc.SetFilter(f['filter'], f['value'],
-                             f.get('exclude', False))
-
-        result = sc.Query(query, self.index)
-
-        if result:
-            return result['matches']
-        else:
-            return []
+    weights = {'title': 4, 'content': 3}

apps/search/helpers.py

@@ -1,4 +1,5 @@
 from django.conf import settings
+from django.utils.encoding import force_unicode
 
 import jinja2
 from jingo import register
@@ -44,4 +45,5 @@ def suggestions(context, string, locale='en-US'):
 
     url = u'%s?%s' % (reverse('search'), query_string)
 
-    return jinja2.Markup(markup.format(url=jinja2.escape(url), text=text))
+    return jinja2.Markup(markup.format(url=jinja2.escape(url),
+                                       text=text))

apps/search/sphinxapi.py

@@ -19,6 +19,8 @@
 import re
 from struct import *
 
+# Kitsune customizations
+K_TIMEOUT = 1  # Socket timeout in seconds
 
 # known searchd commands
 SEARCHD_COMMAND_SEARCH  = 0
@@ -195,12 +197,13 @@ def _Connect (self):
                 addr = ( self._host, self._port )
                 desc = '%s;%s' % addr
             sock = socket.socket ( af, socket.SOCK_STREAM )
+            sock.settimeout(K_TIMEOUT)
             sock.connect ( addr )
         except socket.error, msg:
             if sock:
                 sock.close()
             self._error = 'connection to %s failed (%s)' % ( desc, msg )
-            return
+            raise socket.error
 
         v = unpack('>L', sock.recv(4))
         if v<1:

apps/search/tests/test_json.py

@@ -4,48 +4,51 @@
 
 from sumo.urlresolvers import reverse
 
+from .test_search import SphinxTestCase
 
-def test_json_format():
-    """JSON without callback should return application/json"""
-    c = Client()
-    response = c.get(reverse('search'), {
-        'q': 'bookmarks',
-        'format': 'json',
-    })
-    eq_(response['Content-Type'], 'application/json')
-
-
-def test_json_callback_validation():
-    """Various json callbacks -- validation"""
-    c = Client()
-    q = 'bookmarks'
-    format = 'json'
-
-    callbacks = (
-        ('callback', 200),
-        ('validCallback', 200),
-        ('obj.method', 200),
-        ('obj.someMethod', 200),
-        ('arr[1]', 200),
-        ('arr[12]', 200),
-        ("alert('xss');foo", 400),
-        ("eval('nastycode')", 400),
-        ("someFunc()", 400),
-        ('x', 200),
-        ('x123', 200),
-        ('$', 200),
-        ('_func', 200),
-        ('"></script><script>alert(\'xss\')</script>', 400),
-        ('">', 400),
-        ('var x=something;foo', 400),
-        ('var x=', 400),
-    )
-
-    for callback, status in callbacks:
+
+class JSONTest(SphinxTestCase):
+    def test_json_format(self):
+        """JSON without callback should return application/json"""
+        c = Client()
         response = c.get(reverse('search'), {
-            'q': q,
-            'format': format,
-            'callback': callback,
+            'q': 'bookmarks',
+            'format': 'json',
         })
-        eq_(response['Content-Type'], 'application/x-javascript')
-        eq_(response.status_code, status)
+        eq_(response['Content-Type'], 'application/json')
+
+
+    def test_json_callback_validation(self):
+        """Various json callbacks -- validation"""
+        c = Client()
+        q = 'bookmarks'
+        format = 'json'
+
+        callbacks = (
+            ('callback', 200),
+            ('validCallback', 200),
+            ('obj.method', 200),
+            ('obj.someMethod', 200),
+            ('arr[1]', 200),
+            ('arr[12]', 200),
+            ("alert('xss');foo", 400),
+            ("eval('nastycode')", 400),
+            ("someFunc()", 400),
+            ('x', 200),
+            ('x123', 200),
+            ('$', 200),
+            ('_func', 200),
+            ('"></script><script>alert(\'xss\')</script>', 400),
+            ('">', 400),
+            ('var x=something;foo', 400),
+            ('var x=', 400),
+        )
+
+        for callback, status in callbacks:
+            response = c.get(reverse('search'), {
+                'q': q,
+                'format': format,
+                'callback': callback,
+            })
+            eq_(response['Content-Type'], 'application/x-javascript')
+            eq_(response.status_code, status)

apps/search/tests/test_search.py

@@ -9,13 +9,14 @@
 from django.db import connection
 
 from nose import SkipTest
+from nose.tools import assert_raises
 import test_utils
 import json
 
 from manage import settings
 from sumo.urlresolvers import reverse
 from search.utils import start_sphinx, stop_sphinx, reindex
-from search.clients import WikiClient
+from search.clients import WikiClient, SearchError
 
 
 def create_extra_tables():
@@ -174,3 +175,11 @@ def test_category_exclude(self):
                          {'q': 'audio', 'category': -13,
                           'format': 'json', 'w': 1})
         self.assertEquals(0, json.loads(response.content)['total'])
+
+
+def test_sphinx_down():
+    """
+    Tests that the client times out when Sphinx is down.
+    """
+    wc = WikiClient()
+    assert_raises(SearchError, wc.query, 'test')

log_settings.py

@@ -6,7 +6,7 @@
 
 # Loggers created under the "z" namespace, e.g. "z.caching", will inherit the
 # configuration from the base z logger.
-log = logging.getLogger('dj')
+log = logging.getLogger('k')
 
 fmt = '%(asctime)s %(name)s:%(levelname)s %(message)s :%(pathname)s:%(lineno)s'
 fmt = getattr(settings, 'LOG_FORMAT', fmt)