Bug: Spurious error message from Text Label trait in Prototype

In v3.5, Text Labels were prone to “infinite loops” due to syntax errors in the text label or error messages threatening such. For example, this error message from this small test module.

  • Bad Data in Module: Possible infinite loop evaluating expression BasicName.substring(5)

Version 3.6 improves the situation but still generates a spurious error message from the same module.

  • Bad Data in Module: Piece: Trait: Text Label - {BasicName.substring(5)} Source: {BasicName.substring(5)} Error: Expression evaluation error. See the errorlog for more details

Full error log:

Expression Audit:
Source type=Piece Trait
Source name=Piece:
Source description=Trait: Text Label - {BasicName.substring(5)}
Source field=Label text
Source Expression={BasicName.substring(5)}
Error=e: inline evaluation of: ``_xyzzy=_plugh("");’’ : Method Invocation BasicName.substring

I think the error is because the syntax is being evaluated within the prototype without benefit of where the prototype is actually used (i.e BasicName is null in the prototype, so .substring fails as length is less than 6).

Issue #10972 logged.

1 Like

So the module actually works when you run it, just gives you an error message when you edit the prototype? Or does the module also not actually work? I guess you could work around by checking BasicName but then if it didn’t exist returning “” instead? (And are you sure your BasicName will always be at least 5 characters long?)

{ (BasicName == "") ? "" : BasicName }

{ (BasicName.length() < 5) ? "" : BasicName }

That kind of stuff.

Module works and would work in all cases (BasicName is never less than the required length). Yes, I can do the otherwise unnecessary validation and it will not generate a spurious error.

I’m not sure it’s a good thing to have proactive validation like this, as it means that often module devs have to add logic they would otherwise regard as unnecessary. In fact, they might not even be aware of it until later on. Region (in send to location) is another example. Maybe the architecture drives this and the alternative is worse?

It’s actually Java that requires subString have a string of at least that long, otherwise it throws an error. Personally I think of that as a pretty dumb decision on Javas part, but a lot of “core Java” is a bit over-throwy about exceptions IMHO.

Since when there’s no BasicPiece in a Prototype it makes the string “” then it is too short for your substring and that’s what’s causing the problem.

So I think the diagnosis here is “Java functioning as designed”

2 Likes

Sadly, it seems like the way to avoid substring errors is to always call .substring() as myString.substring(Math.max(myString.length(), x)), where x is the length you actually want.

That’s assuming that substring() is sensible enough to return an empty string and not barf when told to return zero characters, that is (I haven’t tested that)!

At least with BeanShell I’m pretty sure it’s impossible for myString to be null, so you don’t have to check for that, too.

Edit: I’m wondering if it might actually make sense to modify BeanShell to automatically change .substring() in this way, as a form of “error-proofing”… On the other hand, adding calls to Math.max() & String.length() for every call to .substring() might add too much overhead.

Edit 2: Of course, that should be Math.min(), not Math.max(). D’oh!