Error 500 when saving preferences with this simple code

🚧 Developers Broken-code
1

Had a strange Error 500 pop up when writing a driver, I managed to narrow it down and made this code from an example driver to reproduce it without all the clutter:

/*
	Generic Component Dimmer
	Copyright 2016, 2017, 2018 Hubitat Inc. All Rights Reserved
    2019-09-07 2.1.5 maxwell
        -refactor declarations
	2018-12-15 maxwell
	    -initial pub

*/
metadata {
    definition(name: "Generic Component Dimmer Test", namespace: "hubitat", author: "mike maxwell", component: true) {
        capability "Light"
        capability "Switch"
        
    }
    preferences {
        input name: "txtEnable", type: "bool", title: "Enable descriptionText logging", defaultValue: true
        input(name: "telePeriod", type: "number", title: "telePeriod", description: "", displayDuringSetup: true, required: false)
    }

}

def getTelePeriod() {
    return (telePeriod != null && telePeriod.isInteger() ? telePeriod.toInteger() : 300)
}

void updated() {
    log.info "Updated..."

    log.warn "description logging is: ${txtEnable == true}"
    def test = telePeriod

}

void installed() {
    "Installed..."
    device.updateSetting("txtEnable",[type:"bool",value:true])
}

void parse(String description) { log.warn "parse(String description) not implemented" }

void parse(List<Map> description) {
    description.each {
        if (it.name in ["switch","level"]) {
            if (txtEnable) log.info it.descriptionText
            sendEvent(it)
        }
    }
}

To reproduce the Error 500:
0. Use this driver

  1. Make sure telePeriod is set to nothing.
  2. Press "Save Preferences"
  3. BAM Error 500

To make Error 500 to NOT appear (version 1):
0. Use this driver

  1. Make sure telePeriod is set to something, eg: 2
  2. Press "Save Preferences"
  3. All saves as expected

To make Error 500 to NOT appear (version 2):
0. Use this driver but change getTelePeriod to this:

def getTelePeriod() {
    return 1 //(telePeriod != null && telePeriod.isInteger() ? telePeriod.toInteger() : 300)
}
  1. Make sure telePeriod is set to nothing.
  2. Press "Save Preferences"
  3. All saves as expected

I would not expect the behavior, it can't be caught with try-catch either...

Not sure who to ping, but pinging: @bravenel and @mike.maxwell

2

its likely the two preference definitions.
remove the preferences that's within the if, then move that input element and the if statement after the existing inputs in the previous preference closure.

3

No, that is not it, still same issue, that was something I forgot there, just tried with it removed, still same issue

@mike.maxwell In fact, I do have a fully functional driver which does have two preference definitions, but that is not the case in the driver where this issue occurred.

4

ok, if this works it's pure luck, certainly not supported.
If I have time later I'll see if I can look at this some more.

1 Like
5

Ok, thank you, no real hurry, now that I know where the problem is, I can avoid it for now... Just thought this might be something that could creep up on someone else as well, and I would really like to know why this is happening. Hence the minimal possible example.

Ok, for what I want to do it is needed, so I hope you don't design it away...

6

@mike.maxwell Sleeping on it I realized where the problem was:

metadata {
    definition(name: "Generic Component Dimmer Test", namespace: "hubitat", author: "mike maxwell", component: true) {
        capability "Light"
        capability "Switch"
        capability "Refresh"
        
    }
    preferences {
        input name: "txtEnable", type: "bool", title: "Enable descriptionText logging", defaultValue: true
        input(name: "telePeriod", type: "string", title: "telePeriod")
    }
   
}

def getTelePeriod() {
    String a = telePeriod
    return true
}

void updated() {
    log.info "Updated..."
    String a = telePeriod
    log.warn "description logging is: ${txtEnable == true}"
}

void refresh() {
    log.info "Refresh Start"
    String a = telePeriod
    log.info "Refresh End"
}

void installed() {
    "Installed..."
    device.updateSetting("txtEnable",[type:"bool",value:true])
}

void parse(String description) { log.warn "parse(String description) not implemented" }

void parse(List<Map> description) {
    description.each {
        if (it.name in ["switch","level"]) {
            if (txtEnable) log.info it.descriptionText
            sendEvent(it)
        }
    }
}

The above crashed on "Save Preferences". It also makes "Refresh" repeat all code TWICE until reaching "telePeriod" and never executing the code AFTER "telePeriod".

By just changing the method name "getTelePeriod" to "getTelePeriod2" or anything else not matching what I would assume being an "internal auto-generated get method", there are no issues. It also work to declare "void getTelePeriod" (defeats the purpose however, I need a return...) instead of "def getTelePeriod" since that doesn't match the "internal auto-generated get method" signature.

What I get from this is, DO NOT declare a get method with a signature matching this pattern for any preferences and then access that preference inside the "get" function. With that said, trying to CALL "getTelePeriod()" without declaring it in code, does generate a "MissingMethodException", so my description of the problem here isn't 100% accurate.

However, if I run code like this:

def getTelePeriod() {
    //String a = telePeriod
    return "local getTelePeriod()"
}

void refresh() {
    log.info "Refresh Start"
    String a = telePeriod
    log.info "Refresh End: telePeriod=${a}"
}

I would get "Refresh End: telePeriod=local getTelePeriod()" instead of the expected "Refresh End: telePeriod=null".

But none of this really explains why if "telePeriod" IS set to something, everything works and I would NOT get "Refresh End: telePeriod=local getTelePeriod()" or any Error 500... So something is different when a Preference isn't set.

I hope this helps someone and at the very least makes it clear what NOT to do :stuck_out_tongue:

7

When you define a preference, that preference gets injected into your script as a property. So you have "telePeriod" defined and you can use that property in your script. The problem starts when you have not yet set a value to that property so it is null.

Groovy likes to be helpful so when it does not find a property with the name "telePeriod" it starts looking for a method with the name getTelePeriod. The Apache Groovy programming language - Style guide

Since you've defined a method with that name, it will get called and I have no doubt that the 500 error you got was actually a stack overflow exception since your getTelePeriod() method will call itself again on the first line

String a = telePeriod // since telePeriod is not defined, call getTelePeriod() again
4 Likes
8

Thank you! That is a much more concise answer than what I produced, I'm still getting used to Groovy, I hadn't written a line of Groovy (or Java) before I got my HE at the end of November last year. I come from a totally different programming background.
I've read that style guide, but I interpreted it differently (and incorrectly, obviously).

The line "Although the compiler creates the usual getter/setter logic, if you wish to do anything additional or different in those getters/setters, you’re free to still provide them, and the compiler will use your logic, instead of the default generated one." would imply, to me, that I should be able to override it. But I guess for an unset property that would not be possible with this method name, as I've experienced.

It would be nice if all not yet set preferences, or set to empty, would be set to null when injected, wouldn't that be more consistent and avoid this issue? Anyway, now it is all very clear and nothing left to wonder about, so thank you again!

I will also look at using IntelliJ and add unit testing to help keeping track of and identify these issues faster. I've seen some suggestions in the forum and I guess it's time to look at that.