SketchUp / api-issue-tracker

Public issue tracker for the SketchUp and LayOut's APIs
https://developer.sketchup.com/
38 stars 10 forks source link

Passing nil back to UI::HtmlDialog callback produces SystemStackError #112

Open DanRathbun opened 6 years ago

DanRathbun commented 6 years ago

SketchUp Ruby API Issue

Bug Report

1. SketchUp Version: 2018 2. OS Platform: Windows 8.1

Description:

Passing "nil" or ["nil"] back to UI::HtmlDialog callback produces SystemStackError with a "stack level too deep" message.

In some cases we can get "run away" dialogs that just re-appear again after closing one. Then we must kill the SketchUp application process.

Most times, if I close the dialog after getting the stack error and then issue the same command that creates the same class dialog again, SketchUp 2018 crashes WITHOUT a BugSplat.

Now my dialogs are running with a block attached to the call that is run when the dialog closes and the values return, and sometimes that call is called by a method that also has a block that will be in turn yielded. This situation of multiple nested yields might trigger the stack error when the nils are evaluated.

Expectation:

I expected nothing bad to happen, and that the param would be nil, or [nil] on the Ruby-side within the callback block.

I have done this for the "ready" callback with UI::WebDialog and the callback mechanism had no problems. Ie ...

window.location = "skp:ready@nil";

So of course the basic workaround is "don't do this". Ie, if we do not intend to use a callback parameter at all, just don't pass one.

So for the conditional block that fires the readystate callback in a UI::HtmlDialog object, I removed all "dummy" arguments and this strange error goes away.

It strikes me as weird that passing a string of "nil" would create such a bad exception.


On the JavaScript side we have this function and an event listener that will fire off the ready callback ...

    // A chameleon function to send response back to the Ruby-side.
    var send_to_ruby = function (callback,arg) {
        action_in_progress = true;
        if ( typeof sketchup !== "undefined" ) {
            switch (callback) {
            case 'ready':
                sketchup.ready(arg); // calling: sketchup.ready(); is the "fix"
                break;
            case 'clicked':
                sketchup.clicked(arg);
                break;
            case 'cancel':
                sketchup.cancel(arg);
                break;
            };
        } else {
            window.location = "skp:"+callback+"@"+arg;
        }
    }

    document.addEventListener("DOMContentLoaded", function(e) {
        ready();
    });

In some dialogs with decision buttons (using standard labels and button return codes, ie OK = 1, Cancel =2, etc.,) sending nil back to the ready callback causes an exception ...

    var ready = function () {
        send_to_ruby( 'ready', "nil" );
    }

In other dialogs, all the callbacks are expecting arrays, so the following causes an exception ...

    var ready = function () {
        send_to_ruby( 'ready', ["nil"] );
    }

And on the Ruby-side the callback does not even use the paramsparameter, but in debug mode will inspect it ...

  @window.add_action_callback('ready') { |action_context,params|
    json_options = JSON.pretty_generate(dialog_options)
    if debug? && verbose?
      puts "Sample::Plugin - callback :ready"
      puts "  action_context: #{action_context.inspect}"
      puts "  parameters: #{params.inspect}"
      puts "- options:"
      puts json_options
    end
    # Set a VueJs object's data.options property ...
    @window.execute_script(%[dlg.options = #{json_options}])
  }

Could it be the #inspect call when in debug mode that triggers the exception ?

~

thomthom commented 6 years ago

Just to clarify, you get the SystemStackError when using the new callback type in HtmlDialog, using the sketchup object? And you pass in a string "nil"? sketchup.foo("nil")? Or are you using a JavaScript null? sketchup.foo(null)?

DanRathbun commented 6 years ago

Just to clarify, you get the SystemStackError when using the new callback type in HtmlDialog, using the sketchup object?

YES.

I do not use the skp: protocol with UI::HtmlDialog class because that feature could go away as it is undocumented. And if you look at the example above, I use a duck typing conditional that determines if the sketchup JS object is defined and uses it if it is.

And you pass in a string "nil"? sketchup.foo("nil")?

YES. And also sketchup.ready(["nil"]) when the rubyside is expecting an array.

Or are you using a JavaScript null? sketchup.foo(null)?

NO. Ruby does not know what a null is.


See my edited post above, and the note about calling the dialog creation via multiple method calls and the crashing without BugSplat..

Ie, a command method calls another passing a block, which may itself just pass that block along to the constructor method of a dialog wrapper class, which holds onto the block until the dialog closes and then the block is executed using the return value(s) from the dialog.

thomthom commented 6 years ago

I tried to reproduce this, but I didn't see any SystemOverflowErrors.

Do you have a complete snippet to reproduce?

wd = UI::HtmlDialog.new
html = <<-EOT
<!DOCTYPE html>
<html>
<head>
<title>Issue #112</title>
<script>
function call_su() {
  sketchup.testme('nil')
}
</script>
</head>

<body>
<button onclick="call_su()">Test</button>
</body>

</html>
EOT
wd.set_html(html)
wd.add_action_callback('testme') { |action_context, params|
  p params
}
wd.show
DanRathbun commented 6 years ago

This may have been related to the PP class error. It will be awhile before I have time to retest.