-
Notifications
You must be signed in to change notification settings - Fork 130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Error retrieving version? #420
Comments
I'm not seeing this on mine (I'm running latest master, but there haven't been any changes to any of the version coding since Build 277 anyway...):
Check this file: $config_parms{data_dir}/web/mh_download.html and see if it looks like properly formed JSON. |
Hmm, the file looks OK to me. I have verified this is actually the file that was downloaded at 18:00. A copy of the file is here: |
I just diffed my file with yours and they are identical. I was hoping that it might have been something simple like a file with a HTTP error instead of the JSON file. I also ran the file through JSONlint and the file passed as perfectly valid JSON... Is you JSON perl module up to date? An environmental cause is my best guess since both our MH versions as well as the parsed JSON files are identical. To narrow down the issue, you could try running the same file through a simple perl script run directly from the command line to see if the same error is thrown outside of MH. |
Hey Jared, there is a JSON module that comes with MisterHouse under lib/site. This is version 2.27 and as far as I know MisterHouse prefers the included modules over the system-installed ones so AFAIK we should be using the same version of the module too. Or am I missing something? In the mean time I found back in the logs I had the same error on 8/06, on 14/06 and now today. So it is not a one-off. |
I added the mh.private.ini parameter for full log, giving me this when I trigger the version command:
So definitely JSON module related. I'll see if I can narrow this further down. |
You are right--it is using the internal JSON module in lib/site/JSON/PP.pm Do you see the error every single time you run the trigger manually or are you only seeing it scattered in past log messages? Perhaps those happened to be times when GitHub was down for maintenance and an empty response was returned? Looking at the PP.pm file, the error you are seeing should only be thrown if the data passed to PP_decode_json is completely empty or a NULL reference. It would throw a more specific error if it were being passed poorly-formed or completely non-JSON data... |
This On Tue, Jun 24, 2014 at 12:33 PM, Lieven Hollevoet <[email protected]
|
Scratch that--it looks like they have that error listed several times throughout the module; I was only looking at the first occurrence. |
@krkeegan, agreed; I can't think of any other possible cause other than a different perl version perhaps? In any case, if someone can figure out a good way to run files through these functions and the compare the outputs, I would be glad to submit my results. Until today, I didn't have the mh_release code running, so it's possible that I may eventually get some of these as well if it's only occurring every so often. |
You can force the check to run through the following links on the web Mr. House -> Browse MisterHouse -> "Check Misterhouse version" I can click it any number of times without issue. On Tue, Jun 24, 2014 at 12:54 PM, Jared Fernandez [email protected]
|
@krkeegan Yes, that is the way I triggered it to get the 'extended' error report I pasted earlier. I will look into writing a small test script that just reads the file and calls the decode_json() function so that I can experiment with the included JSON module and a recent one. But that won't be for today. |
I finally had some time to explore this further. Compared to the previous error report I updated MisterHouse to the latest master. I examined the error logs further and noticed that I get a recurring daily error log on the version check. So I wrote this small script to test if I could trigger the error when running the code manually:
I can't trigger the problem with this example script. So I'm suspecting something is wrong with the file_read routine that is used in the version check script (I was using the File::Slurp read_file routine). I'll look further into this when I find time to explore it further. A quick grep on 'file_read' shows that this function is used widely in MisterHouse. |
After realizing that there was a small difference between the code in my example script and the mh_release.pl code I was able to get my script to return the same error as MisterHouse does when checking the version. If in the script that is listed in the previous comment I change
to
then the example script errors out with the same error. Obviously, when I change the use line in mh_release.pl to 'use JSON:PP;' then the script works fine. I don't know if it is OK to change the mh_release.pl script like this because I don't know why the original author did write the 'use' statement the way it is written now. What do the others think? Should I create a pull request for this fix (that apparently only occurs on my system/OS)? |
Fix issue #420 (mh_release.pl errors out on decode_json)
Pull request that fixes this has been merged into master. |
Pull request that fixes this is: #463 |
Hello,
I just noticed the following error in my MisterHouse log:
Could this be related to recent changes in parsing the version info?
Is there anybody who is noticing the same error?
MisterHouse info (running a relatively recent master):
Version: develop-ref Build 277 (129f4f4)
Modified: 2014-05-16 19:20:09 +0200
OS: darwin
Perl: 5.18.2
Best regards,
Lieven.
The text was updated successfully, but these errors were encountered: