Safecast / safecastapi

The app that powers api.safecast.org
44 stars 25 forks source link

Trying to access certain measurements by id crashes (500) production #232

Closed thinrope closed 2 years ago

thinrope commented 8 years ago

Certain URLs cause server crashes: GET "/en-US/measurements/14818258" GET "/en-US/measurements/14253404" GET "/ja/measurements/13462231" GET "/ja/measurements/13591872" GET "/en-US/measurements/14337507" GET "/en-US/measurements/13722674" GET "/ja/measurements/13192660"

[b440db41a31f7cc6c4c7adec349f8243] [x.x.100.197] Started GET "/ja/measurements/13192660" for x.x.100.197 at 2016-02-04 02:14:47 +0000
[b440db41a31f7cc6c4c7adec349f8243] [x.x.100.197] Processing by MeasurementsController#show as */*
[b440db41a31f7cc6c4c7adec349f8243] [x.x.100.197]   Parameters: {"locale"=>"ja", "id"=>"xxxxxx"}
[b440db41a31f7cc6c4c7adec349f8243] [x.x.100.197]   Rendered measurements/show.html.erb within layouts/application (121.8ms)
[b440db41a31f7cc6c4c7adec349f8243] [x.x.100.197] Completed 500 Internal Server Error in 134.3ms
[b440db41a31f7cc6c4c7adec349f8243] [x.x.100.197] 
ActionView::Template::Error (undefined method `drive_import_path' for #<#<Class:0x00000006777558>:0x00000006a04cf8>):
    38: 
    39:   <%- if @measurement.measurement_import.present? -%>
    40:     <dt><%= t('.measurement.measurement_import') %></dt>
    41:     <dd title="<%= @measurement.measurement_import %>">
    42:       <%= link_to @measurement.measurement_import.name, @measurement.measurement_import, alt:@measurement.measurement_import.name %>
    43:     </dd>
    44:   <%- end -%>
  app/views/measurements/show.html.erb:41:in `_app_views_measurements_show_html_erb___3246930199769951423_55605520'
  app/controllers/measurements_controller.rb:49:in `show'

while others work: https://api.safecast.org/measurements/49244429 No idea what is common among the above, but some suggestions:

robouden commented 8 years ago

Kalin..

Any idea where in the app code is the "drive_import_path" is? I did search inside all the files could not find it. I could only find " bgeigie_import_path". regards rob

Regards, Rob Oudendijk Yuka Hayashi http://yr-design.biz http://oudendijk.biz http://about.me/robouden tel +81 80-22605966 Skype: robouden Facebook:robouden http://on.fb.me/QeKw2P twitter:robouden http://bit.ly/RAiSTC

On Thu, Feb 4, 2016 at 3:28 PM, Kalin KOZHUHAROV notifications@github.com wrote:

Certain URLs cause server crashes: GET "/en-US/measurements/14818258" GET "/en-US/measurements/14253404" GET "/ja/measurements/13462231" GET "/ja/measurements/13591872" GET "/en-US/measurements/14337507" GET "/en-US/measurements/13722674" GET "/ja/measurements/13192660"

[b440db41a31f7cc6c4c7adec349f8243] [x.x.100.197] Started GET "/ja/measurements/13192660" for x.x.100.197 at 2016-02-04 02:14:47 +0000 [b440db41a31f7cc6c4c7adec349f8243] [x.x.100.197] Processing by MeasurementsController#show as / [b440db41a31f7cc6c4c7adec349f8243] [x.x.100.197] Parameters: {"locale"=>"ja", "id"=>"xxxxxx"} [b440db41a31f7cc6c4c7adec349f8243] [x.x.100.197] Rendered measurements/show.html.erb within layouts/application (121.8ms) [b440db41a31f7cc6c4c7adec349f8243] [x.x.100.197] Completed 500 Internal Server Error in 134.3ms [b440db41a31f7cc6c4c7adec349f8243] [x.x.100.197] ActionView::Template::Error (undefined method drive_import_path' for #<#<Class:0x00000006777558>:0x00000006a04cf8>): 38: 39: <%- if @measurement.measurement_import.present? -%> 40: <dt><%= t('.measurement.measurement_import') %></dt> 41: <dd title="<%= @measurement.measurement_import %>"> 42: <%= link_to @measurement.measurement_import.name, @measurement.measurement_import, alt:@measurement.measurement_import.name %> 43: </dd> 44: <%- end -%> app/views/measurements/show.html.erb:41:in_app_views_measurements_show_html_erb___3246930199769951423_55605520' app/controllers/measurements_controller.rb:49:in `show'

while others work: https://api.safecast.org/measurements/49244429 No idea what is common among the above, but some suggestions:

  • they simply don't exist (but we should return 404, not crash)
  • they are not inside an approved measurement (so, not inside measurements table yet)

— Reply to this email directly or view it on GitHub https://github.com/Safecast/safecastapi/issues/232.

thinrope commented 8 years ago

Nope, no idea. This is what I saw in the logs, then I tested. I have seen (same?) error clicking on some points from bgeigie imports.

eitoball commented 8 years ago

I think that this is because those measurement models belong to measurement_import model with type 'Drive'. I cannot look at database, I can reproduce same error with the following spec.

# spec/views/measurements/show.html.erb_spec.rb
RSpec.describe 'measurements/show.html.erb', type: :view do
  let(:user) { Fabricate(:user) }
  let(:drive_import) do
    DriveImport.create(
      source: Rails.root.join('spec/fixtures/bgeigie.log').open,
      subtype: 'None',
      cities: 'Tokyo',
      credits: 'John Doe'
    )
  end
  let(:measurement) do
    Fabricate(:measurement,
              user: user, measurement_import_id: drive_import.id)
  end
  let(:ability) { Ability.new(user) }

  before do
    allow(controller).to receive(:current_ability).and_return(ability)
    allow(controller)
      .to receive(:default_url_options).and_return(locale: I18n.locale)
    allow(controller).to receive(:params).and_return(id: measurement.id)

    assign(:measurement, measurement)
  end

  it { expect { render }.to_not raise_error }
end

If you run this spec (RAIL_ENV=test bundle exec rspec spec/views/measurements/show.html.erb_spec.rb), you would get error like:

Failures:

  1) measurements/show.html.erb should not raise Exception
     Failure/Error: it { expect { render }.to_not raise_error }
       expected no Exception, got #<ActionView::Template::Error: undefined method `drive_import_path' for #<#<Class:0x007f9584eaef68>:0x007f9584f07118>> with backtrace:
         # ./app/views/measurements/show.html.erb:41:in `_app_views_measurements_show_html_erb__3576996858302337676_70140134083040'
         # ./spec/views/measurements/show.html.erb_spec.rb:26:in `block (3 levels) in <top (required)>'
         # ./spec/views/measurements/show.html.erb_spec.rb:26:in `block (2 levels) in <top (required)>'
     # ./spec/views/measurements/show.html.erb_spec.rb:26:in `block (2 levels) in <top (required)>'

Finished in 0.91027 seconds
1 example, 1 failure

Failed examples:

rspec ./spec/views/measurements/show.html.erb_spec.rb:26 # measurements/show.html.erb should not raise Exception

Note that DriveImport inherits from MeasurmentImport using single table inheritance (STI).

If you execute query like the following, you should be able to get ids of measurements would cause error like this.

SELECT measurements.id FROM measurements
LEFT JOIN measurement_imports ON measurements.measurement_import_id = measurement_imports.id
WHERE measurement_imports.type = 'DriveImport';
eitoball commented 2 years ago

All URLs in description are fine now. Please re-open this issue if there is a measurement that crashes.