helium / blockchain-http

An http API for the helium blockchain database
https://helium.com
Apache License 2.0
47 stars 18 forks source link

Make requests with positive UTC offsets work as well #351

Closed Anbifa closed 2 years ago

Anbifa commented 2 years ago

Dear Helium Developers,

I was playing around with the API around for a little while and encountered an issue.

I was sending requests to the API with timestamps to receive hotspot rewards or account rewards. In the documentation it says that the required timestamps should be put in the ISO 8601 format. When trying out different requests, I found that the the ones with a positive UTC offset consistently returned error: bad request (even +00:00), while the ones with negative always came back fine.

My first thought was that the offsets would just be ignored all together. But after spending time digging deeper, I actually found API works as it is supposed to for negative offsets, while not working for any positive offsets. Like I can send a request with max_time=2021-11-02T00:00:00-12:00 and receive a response with "timestamp":"2021-11-02T08:42:49.000000Z", which should not be the case if offsets would be ignored.

This inconsistency in the API cost me quite some time to figure out and for some other people maybe as well. Also @BellyButton95 already mentioned the same problem in issue #300 in regards to the positive offsets without being aware that the negative ones are already working.

For all the users & developers located east of UTC, it would be great if you could have a look at this issue again.

P.S.: Great project with a unique value proposition, looking forward to where it is heading in the future

For reference:

madninja commented 2 years ago

We've looked at this s a few times, but timezones are a terrible thing to deal with in an api. The best suggestion I have, beyond apologizing for the delay in responding here, is to make your requests in UTC.. i.e. have your client library do the conversion to UTC to avoid the API trying to interpret the fickle timezone rules.

Anbifa commented 2 years ago

Hi there,

yes, I know time zones are a total pain to deal with. So it is totally understandable to not support this.

But then I would kindly ask you to add this limitation to the documentation in order to keep others from scratching their heads or alternatively keep this issue open.

Thanks for looking into this & cheers

madninja commented 2 years ago

Hi there,

yes, I know time zones are a total pain to deal with. So it is totally understandable to not support this.

But then I would kindly ask you to add this limitation to the documentation in order to keep others from scratching their heads or alternatively keep this issue open.

Thanks for looking into this & cheers

Appreciate the response. Hoping to get some help with PRs to the documentation if people are willing and able