<blockquote>
<p>Do we actually need a separate class?</p>
</blockquote>
<p>I haven't added a new class, <code>OSM::APITimeoutError</code> already existed, and the previous code rescued <code>Timeout::Error</code> to raise <code>OSM::APITimeoutError</code>. So, for the API path at least, the code is much simplified.</p>
<p>I'm not sure why, but <code>Timeout::Error</code> doesn't seem to be a regular, rescue-able exception, otherwise the code in Rails for detecting errors would already work. Have a look at the <a href="https://gist.github.com/zerebubuth/8211e07e9f592e5b8146dea2344906b7">gist I put together</a> for more details.</p>
<blockquote>
<p>Also, do we need to do it for the web timeout?</p>
</blockquote>
<p>It's less important, I agree. Some web methods do alter the database and might suffer from the same problem if they're able to partially commit without breaking a database constraint. Anywhere there's a <code>save!</code> in a loop would do it. I'd rather be safe, and add a few extra lines of code, than find out this introduces a race condition somewhere.</p>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/openstreetmap/openstreetmap-website/pull/1381#issuecomment-263361163">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/ABWnLUhMBbPIy9Nj6j_L_ORUjzGCyZBtks5rCyTygaJpZM4K9s2S">mute the thread</a>.<img alt="" height="1" src="https://github.com/notifications/beacon/ABWnLZkefHfy4INRBsO0Dj6OvQg0DImYks5rCyTygaJpZM4K9s2S.gif" width="1" /></p>
<div itemscope itemtype="http://schema.org/EmailMessage">
<div itemprop="action" itemscope itemtype="http://schema.org/ViewAction">
  <link itemprop="url" href="https://github.com/openstreetmap/openstreetmap-website/pull/1381#issuecomment-263361163"></link>
  <meta itemprop="name" content="View Pull Request"></meta>
</div>
<meta itemprop="description" content="View this Pull Request on GitHub"></meta>
</div>

<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/openstreetmap/openstreetmap-website","title":"openstreetmap/openstreetmap-website","subtitle":"GitHub repository","main_image_url":"https://cloud.githubusercontent.com/assets/143418/17495839/a5054eac-5d88-11e6-95fc-7290892c7bb5.png","avatar_image_url":"https://cloud.githubusercontent.com/assets/143418/15842166/7c72db34-2c0b-11e6-9aed-b52498112777.png","action":{"name":"Open in GitHub","url":"https://github.com/openstreetmap/openstreetmap-website"}},"updates":{"snippets":[{"icon":"PERSON","message":"@zerebubuth in #1381: \u003e Do we actually need a separate class?\r\n\r\nI haven't added a new class, `OSM::APITimeoutError` already existed, and the previous code rescued `Timeout::Error` to raise `OSM::APITimeoutError`. So, for the API path at least, the code is much simplified.\r\n\r\nI'm not sure why, but `Timeout::Error` doesn't seem to be a regular, rescue-able exception, otherwise the code in Rails for detecting errors would already work. Have a look at the [gist I put together](https://gist.github.com/zerebubuth/8211e07e9f592e5b8146dea2344906b7) for more details.\r\n\r\n\u003e Also, do we need to do it for the web timeout?\r\n\r\nIt's less important, I agree. Some web methods do alter the database and might suffer from the same problem if they're able to partially commit without breaking a database constraint. Anywhere there's a `save!` in a loop would do it. I'd rather be safe, and add a few extra lines of code, than find out this introduces a race condition somewhere."}],"action":{"name":"View Pull Request","url":"https://github.com/openstreetmap/openstreetmap-website/pull/1381#issuecomment-263361163"}}}</script>