Opened 8 years ago

Closed 8 years ago

#5435 closed enhancement (fixed)

Catch bad comment links to prevent server errors

Reported by: ayleph Owned by:
Priority: minor Milestone:
Component: programming Keywords: comment, link, error, 404
Cc: tsyesika Parent Tickets:


While debugging #5376, we noticed that attempting to access a comment which doesn't exist causes a server error. Ideally, we'd catch this and throw a 404 or just show the media page instead.

Here's an example to illustrate the issue, but these links may go away soon :)

There's an actual comment at this link. Following it works as expected.

Here I fat-fingered manually typing in the link, and I get a server error.

Here's what the error looks like.

Error - <type 'exceptions.AttributeError'>: 'NoneType' object has no attribute 'id'
File '/path/to/mediagoblin/lib/python2.7/site-packages/Paste-', line 144 in __call__
  app_iter = self.application(environ, sr_checker)
File '/path/to/mediagoblin/mediagoblin/', line 342 in __call__
  return self.call_backend(environ, start_response)
File '/path/to/mediagoblin/lib/python2.7/site-packages/Werkzeug-0.9.6-py2.7.egg/werkzeug/', line 588 in __call__
  return, start_response)
File '/path/to/mediagoblin/mediagoblin/', line 276 in call_backend
  return self._finish_call_backend(request, environ, start_response)
File '/path/to/mediagoblin/mediagoblin/', line 318 in _finish_call_backend
  response = controller(request)
File '/path/to/mediagoblin/mediagoblin/', line 47 in wrapper
  return controller(request, *args, **kwargs)
File '/path/to/mediagoblin/mediagoblin/', line 210 in wrapper
  return controller(request, media=media, *args, **kwargs)
File '/path/to/mediagoblin/mediagoblin/', line 170 in wrapper
  return controller(request, page=page, *args, **kwargs)
File '/path/to/mediagoblin/mediagoblin/user_pages/', line 135 in media_home
  mark_comment_notification_seen(comment_id, request.user)
File '/path/to/mediagoblin/mediagoblin/notifications/', line 75 in mark_comment_notification_seen,
AttributeError: 'NoneType' object has no attribute 'id'

Change History (2)

comment:1 by ayleph, 8 years ago

If instead of typing in a comment ID which doesn't exist you type in a comment ID which does exist but isn't attached to this media, the media page is displayed with no comment highlighted. This seems acceptable to me. Here's a patch which replicates that behaviour when a non-existent comment ID is passed.

diff --git a/mediagoblin/notifications/ b/mediagoblin/notifications/
index 9d2f2b7..d554de2 100644
--- a/mediagoblin/notifications/
+++ b/mediagoblin/notifications/
@@ -71,6 +71,11 @@ def mark_notification_seen(notification):

 def mark_comment_notification_seen(comment_id, user):
     comment = Comment.query.get(comment_id)
+    # If there is no comment, there is no notification
+    if comment == None:
+        return
     comment_gmr = GenericModelReference.query.filter_by(,

comment:2 by ayleph, 8 years ago

Resolution: fixed
Status: newclosed

Fixed as of 289826d

Note: See TracTickets for help on using tickets.