search

gravitylow / Updater

Easy automatic updates for Bukkit
MIT License
35 stars 41 forks source link

Return an unknown release type if getting failed #17

Closed ghost closed 9 years ago

ghost commented 10 years ago

Returning null is a silly idea. With this commit, it returns ReleaseType.UNKNOWN.

gravitylow commented 10 years ago

My thinking in this is that this should technically never occur unless A) A new release type is added (unlikely) B) The update fails, in which case the UpdateResult will reflect that

ghost commented 10 years ago

Well, unlikely but possible.

therealspoljo commented 10 years ago

Actually its returning null for few plugins I have installed on my server (AntiCheat (I have renamed it to vKitPvPNoHax :P ) and Vault (I think...)). Here's the error: http://pastie.org/pastes/8756873/text

stuntguy3000 commented 10 years ago

^

I have been noticing that every so often as-well.

ghost commented 10 years ago

Can confirm that happens.

gravitylow commented 10 years ago

That's not odd for an error with the Updater. The idea, however, is that under normal circumstances it never returns null. Null is a placeholder before the loop occurs to find out which it is, as it's far, far more unlikely than I think you would imagine for their to be more release types added to DBO.

If the update checking process itself fails, the result of it is going to be failed and the rest of the information through the getters WILL return null because there is no information there. You should be checking the result before you go on to try and get these things.

This may seem like a small issue but it's a deviation from the functionality of the rest of the system and it carries the implication that there are "unknown" release types (because it exists in the API). These are things I'm not really convinced I want to do.

stuntguy3000 commented 10 years ago

The thing is, the curse server goes down a lot, and I see this error all the time.

gravitylow commented 10 years ago

Again, that's an issue that's out of my control, and if you want to check the information from the update checking process, you should check the update result first. That's why it's there, and contains so many detailed error cases.

I'm not going to make this inconstant with the existing functionality. It'd be silly to return the string "unknown" as a default value for the rest of the information, and it doesn't make sense to do the same thing here.