Fix AJAX endpoints returning HTML instead of JSON on transient errors

deploy/templates/storymap.conf

@@ -56,6 +56,7 @@ server {
       proxy_set_header Host $host;
       proxy_set_header X-Real-IP $remote_addr;
       proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
+      proxy_read_timeout 95s;
     }
   {% endif %}
 }

deploy/templates/storymapjs.service.j2

@@ -14,6 +14,7 @@ EnvironmentFile={{ env_file }}
 # Never reaches the OOM threshold. Jitter prevents all 3 workers from recycling at the same instant.
 ExecStart={{ virtualenv }}/bin/gunicorn -b :{{ service_port }} \
           --workers 3 \
+          --timeout 90 \
           --max-requests 200 \
           --max-requests-jitter 50 {{ wsgi_application }}
 
@@ -32,6 +33,8 @@ MemoryMax=2000M
 
 # Why these numbers
 
+# - --timeout 90 — S3 read_timeout is 60s with up to 5 retry attempts. 90s gives retries room to complete
+#   without gunicorn killing the worker. nginx proxy_read_timeout is 95s to match.
 # - --max-requests 200 — at ~5–10 MB worker drift per heavy save, 200 requests stays well under the 2 GB ceiling
 #   even in the worst case. Tune down to 100 if drift turns out worse.
 # - MemoryMax=2000M — 3 workers × 2 GB = 6 GB cap, which exceeds the 3.7 GB box, so this never bites under normal

docker-compose.yml

@@ -22,7 +22,7 @@ services:
       REDIS_HOST: redis
       RABBITMQ_HOST: rabbitmq
     env_file: .env
-    command: gunicorn -b :5000 --workers 1 --reload --log-level debug storymap.core.wsgi:application --keyfile KnightLabRootCA.key --certfile KnightLabRootCA.crt
+    command: gunicorn -b :5000 --workers 1 --timeout 90 --reload --log-level debug storymap.core.wsgi:application --keyfile KnightLabRootCA.key --certfile KnightLabRootCA.crt
     depends_on:
       - pg
       #- localstack

storymap/api.py

@@ -95,6 +95,34 @@ def https_redirect():
 '''
 
 
+@app.errorhandler(401)
+def handle_401(e):
+    if _is_ajax_request():
+        return jsonify({
+            'error': 'Your session has expired. Please log in again.|'
+                     'Reload this page to sign back in.'
+        }), 401
+    return redirect(url_for('select'))
+
+@app.errorhandler(403)
+def handle_403(e):
+    if _is_ajax_request():
+        return jsonify({
+            'error': 'You do not have permission to perform this action.'
+        }), 403
+    return redirect(url_for('select'))
+
+@app.errorhandler(500)
+def handle_500(e):
+    app.logger.error("Unhandled 500 error: %s", e)
+    if _is_ajax_request():
+        return jsonify({
+            'error': 'A temporary server error occurred. Your changes were not saved.|'
+                     'Please wait a moment and try again.'
+        }), 500
+    return redirect(url_for('select'))
+
+
 @app.context_processor
 def inject_urls():
     """
@@ -154,6 +182,11 @@ def _request_wants_json():
         request.accept_mimetypes[best] > \
         request.accept_mimetypes['text/html']
 
+def _is_ajax_request():
+    if request.headers.get('X-Requested-With') == 'XMLHttpRequest':
+        return True
+    return _request_wants_json()
+
 def _jsonify(*args, **kwargs):
     """Convert to JSON"""
     return app.response_class(json.dumps(dict(*args, **kwargs), cls=APIEncoder),
@@ -319,7 +352,13 @@ def google_auth_verify():
 def get_session_user():
     """Enforce authenticated user"""
     uid = session.get('uid')
-    user = get_user(uid, db=db())
+    if not uid:
+        return None
+    try:
+        user = get_user(uid, db=db())
+    except Exception as e:
+        app.logger.error("get_session_user: DB error for uid=%s: %s", uid, e)
+        raise
     if not user:
         try:
             session.pop('uid')
@@ -341,8 +380,21 @@ def require_user(f):
     """
     @wraps(f)
     def decorated_function(*args, **kwargs):
-        user = get_session_user()
+        try:
+            user = get_session_user()
+        except Exception as e:
+            traceback.print_exc()
+            if _is_ajax_request():
+                return jsonify({
+                    'error': 'A temporary server error occurred. Please try again.'
+                }), 500
+            return redirect(url_for('select'))
         if user is None:
+            if _is_ajax_request():
+                return jsonify({
+                    'error': 'Your session has expired. Please log in again.|'
+                             'Reload this page to sign back in.'
+                }), 401
             return redirect(url_for('select'))
         request.user = user
         kwargs['user'] = user
@@ -358,7 +410,7 @@ def require_user_id(template=None):
     def decorator(f):
         @wraps(f)
         def decorated_function(*args, **kwargs):
-            user = get_session_user()
+            user = request.user
             id = _request_get_required('id')
             if id not in user['storymaps']:
                 error = 'You do not have permission to access to this StoryMap'
@@ -370,7 +422,6 @@ def decorated_function(*args, **kwargs):
                     return render_template('select.html', user=user, error=error, selector_message=message)
                 else:
                     return jsonify({'error': error})
-            request.user = user
             kwargs['user'] = user
             kwargs['id'] = id
             return f(*args, **kwargs)
@@ -489,6 +540,7 @@ def storymap_update_meta(user, id):
 
 
 @app.route('/storymap/export/')
+@require_user
 @require_user_id()
 def storymap_export(user, id):
     """

storymap/static/js/editor.js

@@ -78,9 +78,44 @@ function _ajax(options, on_error, on_success, on_complete) {
         cache: false,
         dataType: 'json',
         timeout: 45000, // ms
-        error: function(xhr, status, err) { 
+        error: function(xhr, status, err) {
+            debug('ajax error', status, xhr.status, err);
+
+            if (status === 'parsererror') {
+                var responseText = xhr.responseText || '';
+                if (responseText.indexOf('entry_login') > -1) {
+                    _error = 'Your session has expired.|Please reload this page to sign back in.';
+                } else {
+                    _error = 'A temporary server error occurred. Your changes were not saved.|Please wait a moment and try again.';
+                }
+                on_error(_error);
+                return;
+            }
+
+            if (xhr.status === 401) {
+                try {
+                    var data = JSON.parse(xhr.responseText);
+                    _error = data.error || 'Your session has expired.|Please reload this page to sign back in.';
+                } catch(e) {
+                    _error = 'Your session has expired.|Please reload this page to sign back in.';
+                }
+                on_error(_error);
+                return;
+            }
+
+            if (xhr.status === 502 || xhr.status === 503 || xhr.status === 504) {
+                _error = 'The server is temporarily unavailable.|Please wait a moment and try again.';
+                on_error(_error);
+                return;
+            }
+
+            if (status === 'timeout') {
+                _error = 'The request timed out.|Please wait a moment and try again.';
+                on_error(_error);
+                return;
+            }
+
             _error = err || status;
-            debug('ajax error', _error)           
             on_error(_error);
         },
         success: function(data) {

tests/unit_tests.py

@@ -113,3 +113,75 @@ def always_fail_save_json(key_name, content):
     assert body['error_type'] == 'transient failure'
     assert body['error_attempts'] == 2
     assert attempts['count'] == 2
+
+
+@pytest.mark.unit
+def test_require_user_returns_json_401_for_ajax_when_no_session(api_client):
+    """When session has no valid user and request is AJAX, return JSON 401."""
+    client, _ = api_client
+    with client.session_transaction() as sess:
+        sess.pop('uid', None)
+
+    response = client.get(
+        '/storymap/?id=map-123',
+        headers={'X-Requested-With': 'XMLHttpRequest'},
+    )
+    assert response.status_code == 401
+    body = response.get_json()
+    assert 'error' in body
+    assert 'session' in body['error'].lower()
+
+
+@pytest.mark.unit
+def test_require_user_redirects_for_non_ajax_when_no_session(api_client):
+    """When session has no valid user and request is NOT AJAX, redirect to select."""
+    client, _ = api_client
+    with client.session_transaction() as sess:
+        sess.pop('uid', None)
+
+    response = client.get('/storymap/?id=map-123')
+    assert response.status_code == 302
+    assert 'select' in response.headers['Location']
+
+
+@pytest.mark.unit
+def test_require_user_returns_json_500_for_ajax_on_db_error(api_client, monkeypatch):
+    """When DB throws during auth check and request is AJAX, return JSON 500."""
+    client, _ = api_client
+
+    def failing_get_user(uid, db=None):
+        raise Exception('connection refused')
+
+    monkeypatch.setattr(api, 'get_user', failing_get_user)
+
+    response = client.get(
+        '/storymap/?id=map-123',
+        headers={'X-Requested-With': 'XMLHttpRequest'},
+    )
+    assert response.status_code == 500
+    body = response.get_json()
+    assert 'error' in body
+    assert 'temporary' in body['error'].lower()
+
+
+@pytest.mark.unit
+def test_is_ajax_request_with_xhr_header():
+    """Detect AJAX via X-Requested-With header."""
+    with api.app.test_request_context(
+        '/storymap/',
+        headers={'X-Requested-With': 'XMLHttpRequest'},
+    ):
+        assert api._is_ajax_request() is True
+
+
+@pytest.mark.unit
+def test_is_ajax_request_without_xhr_header():
+    """Non-AJAX request should not be detected as AJAX."""
+    with api.app.test_request_context('/storymap/'):
+        assert api._is_ajax_request() is False
+
+
+@pytest.mark.unit
+def test_flask_500_handler_returns_json_for_ajax():
+    """Flask 500 error handler should be registered and return JSON for AJAX."""
+    assert 500 in api.app.error_handler_spec[None]