[BUG] Very minor type-checking issue in the shutdown() function of the salt.modules.win_system module

[arizvisa]

Description
This is a very minor typing issue in salt.modules.win_system, and should likely be noted for the next person who ends up committing to this file.

HUVp16SeYiPt2L50peVt:
    The minion function caused an exception: Traceback (most recent call last):
      File "c:\python37\lib\site-packages\salt\minion.py", line 1870, in _thread_return
        opts, data, func, args, kwargs
      File "c:\python37\lib\site-packages\salt\executors\direct_call.py", line 12, in execute
        return func(*args, **kwargs)
      File "c:\python37\lib\site-packages\salt\modules\win_system.py", line 315, in shutdown
        message = message.decode("utf-8")
    AttributeError: 'int' object has no attribute 'decode'

So, this is actually a universal issue and it's mostly because Python's lack of a typesystem forces each implementor to explicitly validate or "cast" the types that they're given as parameters. This is usually fine for internally called functions, but as saltstack exposes these functions directly to the user.. they should be type-checked so that users don't need to know Python in order to resolve these issues.

There's an enforcement of the parameter's type at the top of the function, but unfortunately it's only for Python2 which will soon go away. Whenever it does, that'll likely be a good time to fix issues similar to this.

    if six.PY2:
        message = _to_unicode(message)
   ...

However if these types of bugs are incredibly common, a universal issue could likely be discussed (probably as an SEP) after the ports-to-master project is completed and the code-base stabilizes. These issues can likely be remedied with a combination of two decorators. One decorator which validates types passed as parameters (and maybe the result), and another decorator which enforces the types of the parameters that are passed in from the user. If performance is a concern, some of them could be even disabled during production or affected by the loglevel.

I've done something close to the enforcement decorator in another project (https://github.com/arizvisa/ida-minsc/blob/master/base/_utils.py#L968) as the project is similar to salt in that internally defined functions are exposed to the user as part of the interface. This needed implicit string conversions to be done on the user's input before the function used them, and since the function was being directly exposed there wasn't any other way to wrap it before the user would dispatch into it.

An example for a decorator for shutdown() could look like @salt.utils.decorators.verify(message=str, timeout=int), or @salt.utils.decorators.result_dict(ret_code=int, name=str). Then for enforcing a type maybe @salt.utils.decorators.format(message="{!s}".format). Another benefit of doing something like this is that you can parse out the decorator in a test pretty easily with the ast module (or any linter api) in order to extract parameter type information and test all of the functions that are exposed as the "user-interface" get some automated coverage for these types of issues.

This'd squash a large number of my issues such as #55708, #55944, and likely others.

Setup
Just need a windows minion, and it to be running salt w/ Python3.

Steps to Reproduce the behavior
Pretend that you know what you're doing, but instead pass an integral to system.shutdown as the first parameter thinking it's the timeout. Something like the following will do.

$ salt-call system.shutdown 30

Expected behavior
Likely a better error message possibly warning about which parameter is being passed as the incorrect type since it's only being enforced in Python2.

Versions Report

Salt Version:
           Salt: 3001
 
Dependency Versions:
           cffi: 1.12.2
       cherrypy: 17.4.1
       dateutil: 2.8.0
      docker-py: Not Installed
          gitdb: 2.0.5
      gitpython: Not Installed
         Jinja2: 2.10.1
        libgit2: Not Installed
       M2Crypto: Not Installed
           Mako: 1.0.7
   msgpack-pure: Not Installed
 msgpack-python: 0.5.6
   mysql-python: Not Installed
      pycparser: 2.19
       pycrypto: 3.9.8
   pycryptodome: 3.9.7
         pygit2: Not Installed
         Python: 3.7.4 (tags/v3.7.4:e09359112e, Jul  8 2019, 19:29:22) [MSC v.1916 32 bit (Intel)]
   python-gnupg: 0.4.4
         PyYAML: 5.1.2
          PyZMQ: 18.0.1
          smmap: 2.0.5
        timelib: 0.2.4
        Tornado: 4.5.3
            ZMQ: 4.3.1
 
System Versions:
           dist:   
         locale: cp1252
        machine: x86
        release: 8.1
         system: Windows
        version: 8.1 6.3.9600 SP0 

[arizvisa]

Sorry, I should've probably filed this as an enhancement as although I describe a bug, more of the content leans towards what I would consider a potential enhancement. But again, this should be pretty low priority until you guys get to a point where you're doing minor fixes.