Electroid / mojang-api

Bundle multiple Minecraft APIs into a single GET request.
https://api.ashcon.app/mojang/v2/user/Notch
MIT License
242 stars 17 forks source link

Add easiest error handling #37

Closed Rekseto closed 1 year ago

Rekseto commented 3 years ago

That is easiest fix for issue - https://github.com/Electroid/mojang-api/issues/35 Its not perfect but it works and makes some sense at certain point.

But to be clear much easier would be not writing useless helpers for HTTP that are changing structure of error handling atm you are not able to check inside of API function from which reason mojang failed and giving positive information (account not found when it is not true is really dangerous for potential users of this API). That's why I provided simple error handling that introduces in response third state - false if response is set to false that means that it failed. response that is null is reasonable when there is no content provided by external API (mojang) but when mojang responds with error (like when it's down) we set response to false to give substitute for proper error handling.

So my recommendation is to accept this PR and in free time work on removing all of these HTTP method (because it just wrapping old & good fetch API) and just rewire your methods to use it proper

export usernameToUuid = (username, secs = -1) -> 
  time = if secs >= 0 then "?at=#{secs}" else ""
  response = await fetch("https://api.mojang.com/users/profiles/minecraft/#{username}#{time}")

  if response.ok
     result =  await response.json()
     return {success:true,status: response.status,result }
 else 
     return {success:false, status: response.status, result: null}

  {success, status, result } = unless response = await usernameToUuid(name) 

I just provided fast draft how I would see it working. That change would improve error handling and reduce amounts of code o maintain leaving everything for fetch API.

Electroid commented 3 years ago

Thanks for submitting this, will test it this weekend and try to merge.

ghost commented 3 years ago

bump

Rekseto commented 3 years ago

Basically, it should be rewrited as i suggested in this post. That would be much easier than creating fake abstract layers, that makes maintaining project much harder.