varnish / libvmod-rtstatus

Varnish realtime status page.
Other
37 stars 16 forks source link

Gives invalid json when hitrate is '-nan' #14

Closed obliadp closed 8 years ago

obliadp commented 8 years ago

vmod from varnish-plus-vmods-0.20151210-1.el6.x86_64.rpm

{ "uptime" : "0+13:17:34", "uptime_sec": 47854.00, "hitrate": -nan, "load": -nan, "delta": 5.00, "varnish_version" : "varnish-plus-4.0.3r3 revision 7ddde37", [...]

this results in (using jsonlint):

[Error: Parse error on line 4: ...854.00, "hitrate": -nan, "load": -nan, ----------------------^ Expecting 'STRING', 'NUMBER', 'NULL', 'TRUE', 'FALSE', '{', '[', got 'undefined']

obliadp commented 8 years ago

Wrap it in quotes?

aondio commented 8 years ago

Hi obliadp, thanks for reporting. Is this causing severe problems for you, or are you still able to use the json objects?

obliadp commented 8 years ago

Well, it messes with my monitoring. I use ruby's json lib to parse it for some icinga checks and this is the result:

.rbenv/versions/2.2.2/lib/ruby/gems/2.2.0/gems/json-1.8.3/lib/json/common.rb:155:in `parse': 795: unexpected token at '{ (JSON::ParserError)

the library has an option to allow NaN, excerpt from the doc:

"allow_nan: If set to true, allow NaN, Infinity and -Infinity in defiance of RFC 4627 to be parsed by the Parser. This option defaults to false."

Which is fine, I can do that, but it seems dirty.

Also, from rfc 4627: "Numeric values that cannot be represented as sequences of digits (such as Infinity and NaN) are not permitted."

aondio commented 8 years ago

Hi, @adeelshahid is running tests for this issue, he will push a solution as soon as possible.

adeelshahid commented 8 years ago

@aondio is is possible to put quotes " around all json values returned. That way any bug such as the above can be tackled at the UI level, and the parser will not break.

aondio commented 8 years ago

@adeelshahid have you done some work on this? A message from 17 Dec says you would have a look at it.

aondio commented 8 years ago

Hi obliadp, sorry for getting back to you so late. I've just pushed a commit (b2cc54d) that should fix what you reported.

Please let me know if it works as expected.

aondio commented 8 years ago

Hi, I\m going to close this issue now, but feel free to open a new one if the problem persist.