[openstreetmap/openstreetmap-website] Attempt to make timeouts work properly (24f5795)

[Tom Hughes]

Well if I knew what the problem was then I would have done so! That change was based on a test script that I ran in production but I wasn't seeing exactly the same semantics on my development machine possibly because it's ruby 3.2 and production is 3.0 so things may be different.

What I do know is that in at least some cases there is a difference between this:

```ruby
Timeout.timeout(10, Timeout::Error) do
rescue Timeout::Error
end
```

and this:

```ruby
begin
  Timeout.timeout(10, Timeout::Error) do
  end
rescue Timeout::Error
end
```

with the second one sometimes catching a timeout that is missed by the first one and that getting rid of the explicit class seems to change that.

I did also try and write a test - that was actually my first attempt to reproduce it. That's actually hard to do in itself as you can't just make a query go slowly so I had to resort to extracting the timeout code to a library and then tried to write a lib test using `pg_sleep()` in a query but I didn't have much luck reproducing the problem which is why I moved to running variants of this in production:

```ruby
require "/srv/www.openstreetmap.org/rails/config/environment"

def api_call_timeout(&block)
  begin
    Timeout.timeout(10, Timeout::Error, &block)
  end
rescue ActionView::Template::Error => e
  e = e.cause

  if e.is_a?(Timeout::Error) ||
     (e.is_a?(ActiveRecord::StatementInvalid) && e.message.include?("execution expired"))
    ActiveRecord::Base.connection.raw_connection.cancel
    raise OSM::APITimeoutError
  else
    raise
  end
rescue Timeout::Error
  ActiveRecord::Base.connection.raw_connection.cancel
  raise OSM::APITimeoutError
end

params = {
  :bbox=> "-118.1879,33.4086,-117.6889,33.9076",
  :page=> "1" # 526
}

page = params["page"].to_s.to_i
offset = page * Settings.tracepoints_per_page
bbox = BoundingBox.from_bbox_params(params)

ordered_points = Tracepoint.bbox(bbox).joins(:trace).where(:gpx_files => { :visibility => %w[trackable identifiable] }).order("gpx_id DESC, trackid ASC, timestamp ASC")
unordered_points = Tracepoint.bbox(bbox).joins(:trace).where(:gpx_files => { :visibility => %w[public private] }).order("gps_points.latitude", "gps_points.longitude", "gps_points.timestamp")
points = ordered_points.union_all(unordered_points)

api_call_timeout do
  points.offset(offset).limit(Settings.tracepoints_per_page).preload(:trace).each do |point|
    puts point.id
  end
end
```

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/commit/24f579562fbaf1ec9138014b0f2ed8e219eb75f7#commitcomment-138114539
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/openstreetmap-website/commit/24f579562fbaf1ec9138014b0f2ed8e219eb75f7/138114539 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20240131/12ee5f21/attachment.htm>