Begin, Rescue, Ensure

I have a method that requests data from a web API, I need to make sure only one request is in progress at a time because otherwise SketchUp was crashing. I use a flag @awaiting_response, and set it to false after getting the response. Additional requests are queued as a proc to be executed after the response is returned. In addition if there are changes to the model that require a recalculate the @pending_price_change flag is set so the price can be recalculated only once instead of queuing multiple reprice requests. All this works as intended, but sometimes things will get ‘stuck’ and both flags will remain true, so any future calls are effectively prevented. I could set a timer to reset them after a specific time, but that doesn’t seem like a good solution. My understanding was that the code in the ensure block would run no matter what, guaranteeing the flags to be reset, but apparently that isn’t the case.

Could some one look at this code and see how it’s possible for the flags to not be reset?

  def self.get_panel_data(id = 0)
    @spec_dlg.execute_script('calculating_price()')
    unless @token
      get_token { get_panel_data(id) }
      return
    end    
    download_questions unless @answers

    if @quoting_job
      @pending_price_change = true
      return
    end

    #prevent multiple requests - since it causes crashes
    if @awating_response 
      @pending_requests.push(proc { get_panel_data(id) })
      return
    end 
    

    @quoting_job = true
    @awating_response = true 
    begin   
      @t = Time.now
      update_answers
      url = get_credentials['url'] + 'framer/GetPanelData'
      req = Sketchup::Http::Request.new(url, Sketchup::Http::POST)
      req.headers = { 'Content-Type' => 'application/x-www-form-urlencoded; charset=UTF-8', 'Authorization' => 'Bearer ' + @token }

      form_data = 
        [
          ["jobid", 0],
          ["templateid", 50],
          ['answers', answers_as_xml]
        ]
      form_data.push(*doors_data)
      req.body = URI.encode_www_form(form_data)
      @t = Time.now
      req.start do |request, response|  
        begin  
          @req = request
          json = xml_unescape(response.body.gsub('\\r\\n', "\r\n"))
          if json.include?('The Password field is required.')
            get_token
            break
          end
          @frame_data_alt = JSON.parse(json)
          calulate_price
          @spec_dlg.execute_script("update_price(#{@frame_data_alt['EstimateData'].to_json})")
          puts "#{Time.now - @t} seconds to quote building. (alt)"
          show_materials if !@materials_dlg.nil? && @materials_dlg.visible?
        rescue StandardError => e
          @last_error = e
          puts 'Error pricing job. (alt)'
        ensure
          @quoting_job = false
          if @pending_price_change
            @pending_price_change = false
            frame_job
          end
          check_pending
        end
      end
    rescue StandardError => e
      @last_error = e
      puts 'Error pricing job. (alt)'
      @quoting_job = false
      if @pending_price_change
        @pending_price_change = false
        frame_job
      end
      check_pending
    end
  end

  def self.check_pending
    @awating_response = false
    proc = @pending_requests.pop
    proc.call if proc
  end

I don’t think it is the cause, but I was taught (by the Ol’ Pick Axe book I think,) that whenever you make a local method call without arguments and without a receiver (using dot notation,) that you should help out the interpreter by using a blank () parameter list … ie …

# ...
    rescue StandardError => e
      @last_error = e
      puts 'Error pricing job. (alt)'
      @quoting_job = false
      if @pending_price_change
        @pending_price_change = false
        frame_job()
      end
      check_pending()
    end
  end

It speeds thing up a bit because the interpreter does not need to go through the rigmarole of checking the entire scope for a possible local variable of that name. Ie, by using the () you tell Ruby immediately that this is a method reference.

1 Like

Oh… wait a min. Are these local methods or singleton methods?
(The snippet has them shown as singleton methods defined using self. notation.)

You are using local methods call without qualification …

check_pending()

… instead of singleton method calls which require a receiver …

self.check_pending

You can blame Rubocop for teaching me that (‘bad’?) habit.

image

I was prefixing all my method names with self, because other wise I would get an error.

module BC
  def self.method1
    puts 'method1 called'
  end
  
  def method2
    puts 'method2 called'
  end
  
  method1 #< method1 called
  method2 #< NameError: undefined local variable or method `method2' for main:Object
end

a. I’d only use Rubocop with the lightweight ruleset.
There are too many subjective preference rules in there that don’t have good reasoning behind them.

b. I use an extend self call at the top of my extension module to avoid the method call error.

1 Like

OK for now I’m just going to add a 30 second timeout. At least that will keep the user from needing to restart Sketchup.

UI.stop_timer(@request_timeout) if @request_timeout
@request_timeout = UI.start_timer(30.0, false) do
  @quoting_job = false
  check_pending
end

This reminds me of discussion I’ve had with Thomas with regard to the Sketchup::Http::Request class not having built-in timeout or raising Net errors as the standard library Net::Http classes do.

I also resulted in my study on the differences, and the posting of tables in an API tracker issue thread …


I believe that the Net request blocks use the standard library Timeout module and so can return Timeout::Error which is used by the Net::HTTP.max_retries= setter.

It would be nice if the Sketchup::Http::Request class could also have it’s own edition of what the standard lib’s Timeout module and Timeout::Error does.

We cannot use it (the std lib module) as is because it is blocking, but might be able to use a similar pattern.


FYI - General Tutorials on Rescuing Exceptions (... click to expand ...)

https://thoughtbot.com/blog/rescue-standarderror-not-exception

https://andycroll.com/ruby/rescue-specific-errors-avoid-standarderror-do-not-rescue-exception/

https://www.honeybadger.io/blog/a-beginner-s-guide-to-exceptions-in-ruby/

https://www.rubydoc.info/gems/rubocop/RuboCop/Cop/Style/RescueStandardError

1 Like

I’ve cobbled up a boilerplate idea and posted a feature request for an API Timeout feature …

1 Like