Bungie-net / api

Resources for the Bungie.net API
Other
1.22k stars 92 forks source link

IMPORTANT: DestinyPGCRNotFound will return with HTTP Error Code 404 as of 10/30/2018 #731

Open vthornheart-bng opened 6 years ago

vthornheart-bng commented 6 years ago

A heads up for those of you using the Post Game Carnage Report endpoint, we're going to be changing our system to return 404's when PGCRs can't be found.

The 404 will still return a Response object that has DestinyPGCRNotFound as its error code, but if your client is hard coded to react differently to a non-200 error you will want to adjust accordingly. Thank you for your patience, and pardon the dust!

vthornheart-bng commented 6 years ago

Actually, I have spoken prematurely: we may expand the scope of this somewhat. Stand by, I'll have more information tomorrow.

floatingatoll commented 6 years ago

When such a 404 Not Found occurs at whatever endpoint(s) it ends up occurring at, will it still include a JSON response indicating "this is a valid response from the API" somehow? On Wed, Oct 17, 2018 at 4:58 PM Vendal Thornheart notifications@github.com wrote:

Actually, I have spoken prematurely: we may expand the scope of this somewhat. Stand by, I'll have more information tomorrow.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

vthornheart-bng commented 6 years ago

Aye, the normal response body will still be returned with these error codes as we have been for our existing "errors as 200 code messages" approach, so you can still do Application-level error checking!

vthornheart-bng commented 6 years ago

Okay, so after some discussion we're increasing the scope of this. Starting on 10/30, barring unforseen delays, we're going to begin returning non-200 HTTP status codes for platform responses.

Expect 404's when a requested resource can't be found (such as a PGCR not existing, or an Account not being found), 503's if an underlying game server forces us to reject requests (which has, unfortunately been a common problem lately and we've not been handling it well on our side: we're working on that too, and should see improvements in the upcoming release as well), and 500's as a catch-all for most other types of errors at the moment. We'll be looking to expand those as well to return more appropriate errors, but for now doing this should hopefully be a helpful first step both for our purposes and for those of you who might want to catch more meaningful HTTP Status Codes.

We will indeed continue to pass the normal response body, so you can still do application-level error checking. But if your code is assuming that a 200-level status code will always be returned, you will want to update your implementation appropriately. Thank you, and hopefully this will be better for everyone after this transition period!

ckhicks commented 6 years ago

That's a really good note: status codes will now reflect more than just the server response type, but also give an early indication as to the context of the query being performed. Thanks!

vthornheart-bng commented 6 years ago

One more important update:

TL;DR we're only going to return 404 for PGCRs on 10/30. The other status code changes will come about a month later, to give the latest version of our app time to gain traction.

We have recognized in our testing that the old version of our apps don't properly support real HTTP error status codes in many situations. The newest version of the app coming out this week is going to resolve that issue, but old versions of our app in the wild will likely take a little bit before the majority of the userbase upgrades them.

As a result, we're going to wait about a month before adding the non-200 status codes to anything other than PGCRs. Sorry for the delay! But any prep work you are doing now will be super valuable for when we turn it on for everything.