search

esi / esi-issues

Issue tracking and feature requests for ESI
https://esi.evetech.net/
209 stars 23 forks source link

404's for deleted character_id's break all references to deleted characters #543

Closed cvweiss closed 7 years ago

cvweiss commented 7 years ago

Bug

When attempting to get information on a character that has been deleted the /v4/characters/charID/ endpoint returns a 404

Request

https://esi.tech.ccp.is/v4/characters/2112148223/

Response

HTTP Code 404

Expected

Basic information about the character, including the fact that they are now in the Doomheim corp (having been deleted). This behavior is seen with the XML API using this endpoint: https://api.eveonline.com/eve/CharacterInfo.xml.aspx?&characterId=2112148223

Previous XML or CREST route

https://api.eveonline.com/eve/CharacterInfo.xml.aspx?&characterId=2112148223

Additional information

https://zkillboard.com/character/2112148223/

gitAskur commented 7 years ago

...I'm not very inclined to spool up tracking objects for deleted characters to tell you the truth.

But I guess I'll solve it some day. How important is being able to resolve deleted characters, in the grand scheme of things?

cvweiss commented 7 years ago

aquahead asked me to put this here...

How about an error code besides a 404 to indicate that yes, this character may have once existed but doesn't any longer.

cvweiss commented 7 years ago

As for importance, it'd be nice to know that the character has been deleted somehow rather than getting the ambiguous 404. Tools that used the character, or still keeps data on the character (such as killboards) can reflect the proper status of the character.

If the only condition for a previously existing character to generate a 404 is that its been doomheim'ed, then I guess that is something we could work with?

gitAskur commented 7 years ago

We could maybe do a different error code. But honestly I´d just much rather not expose that through get character.. the character is not there to get.

But I'll take a look at the client capacity with deleted characters tomorrow. And whatever happens you'll always be able to lookup the name of deleted characters for reference reasons.. but wether or not you can do that through character get rather than something else is up for debate.

gitAskur commented 7 years ago

But if we change error codes I´d probably throw you a 410. I'm just not quite sure how much work it is to make that consistent for everything.

cvweiss commented 7 years ago

I'm pretty flexible, I'll work with whatever you give us! Thanks!

deadlybulb commented 7 years ago

+1 for making this a 410. In an ideal world, you'd be able to discover if the char once existed, but there's usually context for figuring that out. In the case of the killboard example, if there were once entries for a char but now a 410 shows up, one could infer the char was deleted.

cvweiss commented 7 years ago

There is one edge case which may be fixed by creating another endpoint allowing us to look up removed characters:

In game, we'd be able to do a Show Info even on the deleted character from a linked killmail.

gitAskur commented 7 years ago

You should absolutely always be able to look up names of deleted characters, just like the in game client displays those in any logs rather than some ID or fallback string. This specific endpoint is not the best candidate for that however, and I assume you do not actually need most of the data returned from it if you are just trying to display human readable log information. It's obviously useful to know that you pod killed a character named Prism X, who then deleted his character and presumably ragequit the game, but you don't really care whether he's a True Gallente and that his biography is filled with socialist propaganda.

Robbilie commented 7 years ago

@gitAskur which fields of the character model that are mandatory are not retrievable for doomheimed chars?

https://esi.tech.ccp.is/latest/#!/Character/get_characters_character_id

i would very much appreciate doomheimed chars to be normally returned by that endpoint, corporation_id being 1000001 is enough of an indicator that this char is doomheimed and there is no way of it coming back to life right?

Robbilie commented 7 years ago

sorry that i am late to the party, only realized me hitting the 404 too a few hours ago :D

gitAskur commented 7 years ago

As I said above: "This specific endpoint is not the best candidate for [that.]".

Honestly, servicing dead characters is technically problematic for us as it stands. I understand that does not sound intuitive, especially when I tell you that literally all the data is still there so it's certainly not a problem with data retrieval. Couple that with the fact that it was not a problem for the XML API and I can totally see why you'd be frustrated with my stance. All I can say is that the XML API and the EVE Online server are radically different beasts with radically different needs.

But perhaps this endpoint is the best candidate. I really need to sit down and think about how we are going to handle this without starting to track changing session variables for deleted characters, or having queries about deleted characters always causing db dips and thus become dangerous when people with zero caching strategies or request back-off strategies become interested in them.

So it really isn't a case of me wanting to mess with any ESI data model, or turning mandatory fields into quantum states dependent on the cat being dead or not. I just genuinely do not quite know how to standardize the response format with regards to the different external ESI endpoints and internal actions they use and other factors.

I'd also really like to do this whole "Measure twice and cut once" thing here. If I just knee jerk fix shit up into some jerry rigged solution, that temporary solution will become the permanent one as any attempts to bringing in new standards will be met with boos and jeers.

So I'm keeping this issue on me, and as something I need to address asap. But it might not be today or tomorrow because I literally have no other boxes than my "asap" box because why would anyone care about things that are in other boxes when you have an "asap" box?!

gitAskur commented 7 years ago

Changing name of issue to be less specific so I don't forget all of this and just solve it for this single instance.

cvweiss commented 7 years ago

Sounds good!

In the meantime, I've setup my code to put the character into Doomheim if it 404s on this endpoint. To help reduce the 404's and db data dips I'll also set up my code to not poll characters in Doomheim anymore. I used to do that, but at one time there was an influx of zombies and many characters that were doomheim'ed were back in regular corporations...

gitAskur commented 7 years ago

Turns out I could not leave this for today and after tomorrows downtime, barring any unforeseen accidents and other nonsense, you'll have 410s coming for deleted characters.

This will apply to a lot of calls that are currently responding with 404s. Probably all of them except the one you are really hoping will. Also PoSs will start shootings blues under highly narrow and specific circumstances that still somehow applies to everyone who matters (so no worries, not you).

Hah! Mondayz!

aquarhead commented 7 years ago

Just so that everyone knows: after the fix is deployed, NPC and deleted characters will just like live player characters. so you can resolve names etc.. they just won't change anymore (hopefully).

Non-exist (not created) characters will still be 404.