500 on MR because a nil DiffNote's diff_line isn't accounted for
Zendesk (internal link): https://gitlab.zendesk.com/agent/tickets/99353
This customer is running 10.8.4-ee and is getting the following 500 error when trying to view a merge request:
Started GET "/<namespace>/<project>/merge_requests/6" for <IP> at 2018-06-27 08:42:49 -0400
Processing by Projects::MergeRequestsController#show as HTML
Parameters: {"namespace_id"=>"<namespace>", "project_id"=>"<project>", "id"=>"6"}
Completed 500 Internal Server Error in 396ms (ActiveRecord: 44.8ms | Elasticsearch: 0.0ms)
ActionView::Template::Error (undefined method `index' for nil:NilClass):
17: %table
18: - if expanded
19: - discussions = { discussion.original_line_code => [discussion] }
20: = render partial: "projects/diffs/line",
21: collection: discussion.truncated_diff_lines,
22: as: :line,
23: locals: { diff_file: diff_file,
app/models/concerns/discussion_on_diff.rb:44:in `truncated_diff_lines'
app/views/discussions/_diff_with_notes.html.haml:20:in `_app_views_discussions__diff_with_notes_html_haml___2271884178805127179_70054995643080'
app/views/discussions/_discussion.html.haml:52:in `_app_views_discussions__discussion_html_haml__1728541228339610768_70054930286920'
app/views/shared/notes/_notes.html.haml:6:in `block in _app_views_shared_notes__notes_html_haml___1948692214257223412_70054893354300'
app/views/shared/notes/_notes.html.haml:2:in `each'
app/views/shared/notes/_notes.html.haml:2:in `_app_views_shared_notes__notes_html_haml___1948692214257223412_70054893354300'
app/views/shared/notes/_notes_with_form.html.haml:6:in `_app_views_shared_notes__notes_with_form_html_haml___3230466369440081720_70054891283080'
app/views/projects/merge_requests/_discussion.html.haml:11:in `_app_views_projects_merge_requests__discussion_html_haml__4544443769467627885_70054887719340'
app/views/projects/merge_requests/show.html.haml:92:in `_app_views_projects_merge_requests_show_html_haml___1445104828519208308_70054914939540'
app/controllers/projects/merge_requests_controller.rb:56:in `block (2 levels) in show'
app/controllers/projects/merge_requests_controller.rb:41:in `show'
lib/gitlab/i18n.rb:50:in `with_locale'
lib/gitlab/i18n.rb:56:in `with_user_locale'
app/controllers/application_controller.rb:363:in `set_locale'
lib/gitlab/middleware/multipart.rb:95:in `call'
lib/gitlab/request_profiler/middleware.rb:14:in `call'
ee/lib/gitlab/jira/middleware.rb:15:in `call'
lib/gitlab/middleware/go.rb:17:in `call'
lib/gitlab/etag_caching/middleware.rb:11:in `call'
lib/gitlab/middleware/read_only/controller.rb:28:in `call'
lib/gitlab/middleware/read_only.rb:16:in `call'
lib/gitlab/request_context.rb:18:in `call'
lib/gitlab/metrics/requests_rack_middleware.rb:27:in `call'
lib/gitlab/middleware/release_env.rb:10:in `call'
It looks like it's failing on this line, where diff_line
is nil
so index
can't be called on it.
In the rails console, we confirmed the MR has 10 discussions, none of which are a LegacyDiffDiscussion
. However, there are a few DiffNote
objects:
irb(main):004:0* mr.discussions.each{|d| d.notes.map{|n| puts "#{n.class}: #{n.id}" }}; nil
Note: 633205
Note: 633207
Note: 633213
Note: 633339
DiffNote: 633397
DiffNote: 633400
DiffNote: 633401
DiffNote: 633402
Note: 633403
Note: 633816
=> nil
...and they're all missing diff_line
entries:
irb(main):019:0* diff_note = DiffNote.find(633397); diff_note.diff_line
=> nil
irb(main):020:0> diff_note = DiffNote.find(633400); diff_note.diff_line
=> nil
irb(main):021:0> diff_note = DiffNote.find(633401); diff_note.diff_line
=> nil
irb(main):022:0> diff_note = DiffNote.find(633402); diff_note.diff_line
=> nil
Earlier in the method, here, we catch it if it's nil
AND comes from a LegacyDiffNote
object, but don't account for when it's nil
and comes from a DiffNote
object. This looks like a bug.
Please see the ticket (an internal link) for more: https://gitlab.zendesk.com/agent/tickets/99353