Bungie-net / api

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

modeType 40 (social) generates Unhandled Exception Error #227

Open vpzed opened 6 years ago

vpzed commented 6 years ago

modeType 40 (social) generates an "UnhandledException" error on the Destiny2.GetHistoricalStats endpoint. This modeType is listed in the current manifest (edf4d47841acbd74f02ae136523ecc37) in DestinyActivityModeDefinition.

Even if there isn't data I would expect the endpoint to return:

{
    "Response": {
        "social": {}
    },
    "ErrorCode": 1,
    "ThrottleSeconds": 0,
    "ErrorStatus": "Success",
    "Message": "Ok",
    "MessageData": {}
}

However a call like:

https://www.bungie.net/Platform/Destiny2/1/Account/4611686018453017957/Character/2305843009263913960/Stats/?periodType=2&groups=1&modes=40

returns:

{
    "ErrorCode": 3,
    "ThrottleSeconds": 0,
    "ErrorStatus": "UnhandledException",
    "Message": "We've encountered an error, please try again later.",
    "MessageData": {}
}

This error is returned if 40 is anywhere in the modes list (even if other working modes are included).

sgtfrankieboy commented 6 years ago

I'm assuming because mode 40 is a unique case. I would personally ignore it completely and think it as not existing, they used to fall under mode 0 in D1.

If nothing has changed from how they were handled in Destiny 1, they do not have any useful information. Only the players character / account ids. If you request a single PGCR you will get thousands of player entries as they are long lived sessions.

I think the error makes a bit of sense as there is data available for mode 40, its just something that has been hidden. The only way to get a Social match is by guessing its ID.

vpzed commented 6 years ago

@sgtfrankieboy I guess we disagree on this point then. I don't think the API specification should have an endpoint that says this query string parameter accepts a comma separated list of values from this enum, and then generate errors if a value that the spec enum provides is used on that endpoint. If the developer should "ignore it completely" then it shouldn't be in the spec enum.

I feel the same way about listed endpoints in the spec that require application scopes that are not available to application developers. If an endpoint is impossible to use then don't include it in the application developer specification.

Putting gotcha's in the specification just doesn't make sense to me. But that's just my opinion of course.

iBotPeaches commented 6 years ago

Should an endpoint return an UnexpectedError? Probably not.

If you are trying to use social endpoints, your best bet is PGCR (numerical increase) as hinted at by @sgtfrankieboy. With context of the goal at hand we probably can lend some advice with what to do. If you are just hitting endpoints with all possible values, then nevermind :)

As for the spec. Seeing how we communicated in D1 without a spec, then partial official documentation and a little unorganized Bungie forum this spec was an amazing helpful step up from the previous and greatly appreciated.

It has issues here and there, but I think we'd both agree there are other issues in the API that the finer details of how that spec is generating things is probably not worth the time for Bungie to fix. We as a community find the "gotchas" in the API and make that knowledge known. If by design mode 40 is disallowed from the Stats endpoint then I guess it would be time to look into fixes for the spec to correct this. Thankfully the community can help there as OpenAPI specs are public.

In my experience I am not aware of an OpenAPI way to use an enum, with the exception of certain values. Perhaps I can be proved wrong, I looked a bit in the documentation and found no way.

vpzed commented 6 years ago

Yep. I see this as a spec issue not an API issue per se. It looks like an "auto-generation oversight". Just guessing, but like there is one definition for the modeType list in the source material and the spec generator is referencing that list where ever modeTypes are used by an endpoint, but not applying endpoint specific filters to the list. So the modeType list for the Destiny2.GetHistoricalStats ends up listing a value that shouldn't be there for that endpoint (but might be fine for a different endpoint).

I see the API spec as the law for the API. Right now it says that one can put mode 40 into that endpoint and expect it to work, but that is not the case.

If one had just queried the manifest and put a value in there that wasn't in the spec as a valid value then all bets are off, but I don't think following the spec should generate an error. Again just my opinion.

Fixing something like this is definitely lower priority to me than say getting more endpoints functioning at all - like Clan Leaderboards, Aggregate Activity Stats, Aggregate Clan Stats, Vendors, etc. I wish there was a way to assign a label so that could be clear. As-is I'm just posting what I find and letting the devs assign priority.

vthornheart-bng commented 6 years ago

Yeah, agreed all around - I really want to improve the spec (there's a lot of places where things were true and are now not, were never meant to be true but we typo'd/fumbled on the docs, or where the code generator is just plain producing incorrect output). But indeed, I also agree that getting the non-functioning features working and adding to the raw features takes priority. That's why some of these bugs - mostly spec bugs - will probably not see any love for a bit. But it's good that they're documented here, because when we do have time to loop back to them we can give them the TLC they deserve.

I'll mark this as a spec bug for now. After a bit of ado I'm finally back to working on resolving the problems that have been preventing us from exposing Vendors, and then we'll keep moving forward from there!

I appreciate all the input you all have given, and both your enhancement and bug requests. Though our ability to address them quickly has been hampered and our timelines have been delayed - particularly with my unexpected 2 week hiatus, but also just with the number of issues across the systems - having them documented here and able to be commented on is already doing a great benefit for being able to address and understand what issues people are seeing in the wild. We deeply appreciate it, and though I'm almost positive that we'll never be able to resolve them as quickly as more are created, having it here will let us continue to address them when time permits and will help us to move forward!