Improve error handling

[bennlich]

Right now turtle.forward('dogs') or just turtle.forward(undefined) leaves turtle in a broken state, stranded at NaN, NaN. Instead, turtle.forward('dogs') should raise a ValueError.

I noticed this when editing running code in the landing page tutorial.

[backspaces]

Won't this lead to a huge amount of error checking everywhere? I realize we've decided on a different audience than initially (I.e. JS programmer to novice), but I'd like to minimize the disruption.

We could use defaults for more method arguments, so something like forward(d = 1). Probably should use distance rather than d.

Math.sin('dog') returns NaN for example. It doesn't throw an error, alas, but then STEM folks may not know about try/catch.

If we do want to have special things for the novice user, I think I'd like to take the approach of creating a NL "app" which doesn't use MV split or radians or ... See the list I posted in AS channel on how to get to degrees & headings.

[bennlich]

Here's what I did that made me run into that error:

1. Click run forever on one of the blocks that moves turtles
2. Try changing forward(1) to forward(10). All good.
3. Now try changing to forward(5). When you get to forward(), the turtles positions become NaN, and you can't restore them without resetting the model.

Math.sin('dog') returns NaN for example.

I think returning NaN is fine behavior. But letting the turtle's internal state become NaN is bad because then you can never get out.

Won't this lead to a huge amount of error checking everywhere?

That's a good question. It looks like this also happens with turtle.rotate(). Maybe we can survey and see how many places this would show up.

[backspaces]

Check out the slack chat:
https://redfishgroup.slack.com/archives/C033X4FF8/p1614118299077300

It could be that we solve error checking depending on how we resolve JS <> NL geometry.

Part of me thinks we should leave AS as a JS geometry with simple utils,js helper functions dealing with conversions such as radians<>degrees & euclidean<>compas geometry. If so, it would be easy for these helpers to also do error checking.

Similarly, I'm looking into a transform approach of the 40 functions that need it. They would pass all angles, relative or absolute, thru the utils.js helpers, thus also being able to check them.

But that doesn't solve the positions. But setxy & setxyz could just multiply all their args and if a NaN pops out, we throw an Error.

But I completely agree, it would be great to have deeper error checking. And you get to find most of them with your fresh eyes! Thanks!

[backspaces]

Check out the slack at https://redfishgroup.slack.com/archives/C033X4FF8/p1614118299077300

[bennlich]

@backspaces I did a little user-testing of the homepage tutorial, and this came up again. I think we should figure out a solution before launching the new site.

This problem shows up specifically in a live-update situation, where a snippet of code is getting constantly re-evaluated while being edited.

My specific desire is: prevent turtle x, y from ever becoming NaN due to a call to e.g. turtle.forward() or turtle.rotate() with no args.

The more general way to say it is: prevent the model from ever entering a "bad state" due to an improper function call. A "bad state" is one that can't be recovered from without rebuilding the whole model. But for launch, I think we should just solve the specific case of the turtle NaNs.

setxy & setxyz could just multiply all their args and if a NaN pops out, we throw an Error.

Does this still seem like a reasonable way to go?

[backspaces]

Agreed, wouldn't hurt. Time for AS to grow up!

[backspaces]

Does TypeScript handle errors for you?

[backspaces]

@cody points out TS type checks at compile time so isn't that useful unless the modeler uses TS and we supply type info. Even then, it probably isn't what we want, which is inherently runtime checking.

I guess a set of type utilities may be useful, but because JS pretty much sucks at that (no Integers, too primitive in general)

[bennlich]

If you're worried about performance hits, maybe we could have only a dev build that does these checks?

[stigmergic]

Yes I hear the concern over adding a ton of error checking code, what a PITA. But I agree that given the audience we are targeting (novice) we want to protect them.

I think this calls for a type sanity check--as we are mostly expecting numbers as arguments we could start with just ensuring that the arguments are number like values (Number(arg) !== NaN), throwing an error for a try/catch would be the cleanest dev friendly approach. A user friendly approach might be a noop and emitting a warning to the log.

if (Number(arg) !== NaN) {
  console.warn('argument passed was not a number.  Got this instead: ', arg);
  return;
}

It does add some overhead, I'd be interested in how much.

[bennlich]

I'm monkey-patching Turtle3D in the IDE and tutorial for now:

import Turtle3D from '/agentscript/src/Turtle3D.js'

let check = (d) => {
  if (typeof d !== 'number') {
    throw new Error(`Function expected a number, got ${d}`)
  }
}

Turtle3D.prototype._forward = Turtle3D.prototype.forward
Turtle3D.prototype.forward = function(d) {
  check(d)
  this._forward(d)
}

Ditto for right, left, and rotate.