frebib / nzbget-exporter

NZBGet Prometheus metrics exporter
20 stars 5 forks source link

Fix unmarshaling error when TotalSizeLo < 0 #1

Closed juniorz closed 4 years ago

juniorz commented 4 years ago

For some reason TotalSizeLo values might be negative, causing an error: json: cannot unmarshal number -1 into Go struct field temp.TotalSizeLo of type uint32

See: https://forum.nzbget.net/viewtopic.php?t=3711

frebib commented 4 years ago

Thanks for sending the PR! This is something I fought a lot when writing this, and I ended up fixing the problem by patching NZBGet. I found the documentation somewhat contradictory to the code which didn't help matters. Good job on adding the tests, I'm usually too lazy when it's already working. I think this could actually affect multiple fields so it's probably worth generalising this code and applying it to all values from the API. I'll have a look to see if there are other fields that we should apply this too, as I expect there will be

frebib commented 4 years ago

Thinking about it, I think the best approach here is to use int32 for any values that NZBGet uses int32 for and do the check/conversion just in case. No doubt it will prevent an error or two in the future

frebib commented 4 years ago

Will this cause an unmarshal error if the value is between INT32_MAX and UINT32_MAX? It may be necessary to use int64 to prevent the same issue at the upper end

juniorz commented 4 years ago

Oh. Sorry for the multiple force-pushes, I wasn't expecting such a prompt response.

I have generalized on my fork, and I am adding to this PR. Let me know if you have any other concern.

frebib commented 4 years ago

I'm inclined to say that this isn't something that we should fix with code, but instead fix the underlying problem instead. I'll add a note to the README and also patch spritsail/nzbget branches with the fix