if (thing) thing.dostuff() ... runtime error: Cannot execute null.dostuff(). WTF?

ID:2076942
 
Apr 29 2016, 7:30 am
Resolved
Calling a proc on an invalid value said "Cannot execute null.[proc_name]()" even if the value wasn't null. This has been fixed in several applicable routines for better runtime error reporting.
BYOND Version:510.1338
Operating System:Windows Server 2012 rc2 64-bit
Web Browser:Chrome 49.0.2623.112
Applies to:Dream Daemon
Status: Resolved (510.1341)
This issue has been resolved.
Descriptive Problem Summary:
runtime error: Cannot execute null.Copy().
proc name: New (/obj/item/weapon/grown/New)
source file: growninedible.dm,16

Code Snippet (if applicable) to Reproduce Problem:
/obj/item/weapon/grown // Grown weapons (line 5)
    name = "grown_weapon"
    icon = 'icons/obj/hydroponics/harvest.dmi'
    burn_state = FLAMMABLE
    var/obj/item/seeds/seed = null // type path, gets converted to item on New().

/obj/item/weapon/grown/New(newloc, var/obj/item/seeds/new_seed = null) //line 11
    ..()
    create_reagents(50)

    if(new_seed) //<--- it exists here
        seed = new_seed.Copy() //line 16 runtime error: Cannot execute null.Copy()
    else if(ispath(seed))
        // This is for adminspawn or map-placed growns. They get the default stats of their seed type.
        seed = new seed()
        seed.adjust_potency(50-seed.potency)

    pixel_x = rand(-5, 5)
    pixel_y = rand(-5, 5)

    if(seed)
        for(var/datum/plant_gene/trait/T in seed.genes)
            T.on_new(src, newloc)

        if(istype(src, seed.product)) // no adding reagents if it is just a trash item
            seed.prepare_result(src)
        transform *= TransformUsingVariable(seed.potency, 100, 0.5)
        add_juice()




I'm at a loss.
Apr 29 2016, 7:51 am
I'm unable to get this to happen on 510.1340 using code which should produce the same result.
obj/foo/proc/Copy()
  world << "COPIED"
  return new /obj/foo

obj
  bar
    var/obj/foo/dummy = null

    New(newloc, obj/foo/new_dummy = null)
      ..()

      if(new_dummy)
        dummy = new_dummy.Copy()

mob/Login()
  ..()

  new /obj/bar(src, new /obj/foo)

I can go back to 510.1338 and check.

[Edit]

Yeah, that code works fine in 510.1338 for me, so this seems particular to your case.
Apr 29 2016, 7:53 am
The bug here is the runtime reporting a '5.Copy()' as 'null.Copy()'.

A numeric value is being passed instead of a list, passes the if check because it's not null and then crashes when it's used as a list. So it shouldn't say it is null, but rather that it's the wrong type of variable or something like that.

Surely we can't access objects through their reference values.

new/obj/item/weapon/grown/bananapeel/ specialpeel(get_step(src,turn(usr.dir, 180)), 5) //honk
Apr 29 2016, 7:55 am
We found the cause, it's actually that DM reports numbers as Null sometimes:

eg:
/mob/verb/test_it()
    var/list/L = 5
    L.Copy()

(this is not the code, obviously)

Outputs
"runtime error: Cannot execute null.Copy().
proc name: test it (/mob/verb/test_it)"

This 5 was caused by code from an old refactor.

So really those types of errors should say the actual value (5 in this case) as it would have made fixing the error easier (and would have saved us this erroneous report)
Apr 29 2016, 7:58 am
In response to CrimsonVision
And, yeah, changing the login code to
mob/Login()
  ..()

  new /obj/bar(src, 5)

reproduces the result. Nevertheless, this is not an erroneous bug report. If the debug output is giving improper output, it's still a bug, just a different one that you had thought.
Apr 29 2016, 7:59 am
It's reporting null because you're casting a number as an object type. Pretty sure it's perfectly valid. Do type checking before casting.
Apr 29 2016, 8:00 am
It's not null, though, so it shouldn't say it's null. The purpose of a runtime error report is to help debug, not confuse. Saying "do type checking" accomplishes nothing.
Apr 29 2016, 8:00 am
In response to Super Saiyan X
That's still erroneous, though. E.g.,
var
  a = 5
  obj/b = a
world << b

will output 5 instead of null. The debug output is wrong in any case.
Apr 29 2016, 8:02 am
Yeah we really just want the output fixing here, we know the main bug is just old leftover code pre-refactor, but telling us a 5 is null causes problems (and erroneous bug reports!)

(and yeah by erroneous I mean the fact that we were initially reporting something that is our fault)
Apr 29 2016, 8:04 am
In response to Xxnoob
Xxnoob wrote:
It's not null, though, so it shouldn't say it's null. The purpose of a runtime error report is to help debug, not confuse.
You're right. This only seems to occur with proc calls though, accessing a variable does output the right value in the runtime.

Saying "do type checking" accomplishes nothing.
Sure it does, it tells you to write your code in such a way where you're not assuming the type of the value that is being passed to your function. :)
Apr 29 2016, 8:06 am
In response to Super Saiyan X
Sure it does, it tells you to right your code in such a way where you're not assuming the type of the value that is being passed to your function. :)

I'd actually argue that it failing non-gracefully is the right approach, here. You want to know that you're passing the wrong arguments to your method, or else you can get really confusing bugs down the line. I mean, obviously you can do type-checking and throw an exception with more-explicit details, but doing as is and getting a "loud" DM error works just as well.
Apr 29 2016, 8:10 am
It's proper coding to ensure you're passing the right things. This bug was caused by a refactor and if there was additional checks to stop it from crashing we would still not know about it.

If the code is poorly written it should crash, so you know to write it better or fix it, saving from every crash just ensures code you forget about stays outdated.
Apr 29 2016, 9:18 am (Edited on Apr 29 2016, 9:26 am)
Yep, I can confirm that the proc error messages say "null" instead of the correct value. That's hard-coded, and it shouldn't be.

[edit]
Interestingly, two of the functions where this message appears have no src information to go on, so a proper fix isn't available for those at this time. Two others did have src available. So it's questionable whether the fix I have will address this specific case. It probably will.
Apr 29 2016, 9:27 am
Lummox JR resolved issue with message:
Calling a proc on an invalid value said "Cannot execute null.[proc_name]()" even if the value wasn't null. This has been fixed in several applicable routines for better runtime error reporting.
Apr 29 2016, 9:31 am
Thanks Lummox.
May 1 2016, 3:57 pm
This is gonna be super helpful tracking down some of our null.x runtimes