manifest: parse per section, and drop the unused CA task registry - #54
Open
joelhalen wants to merge 1 commit into
Open
manifest: parse per section, and drop the unused CA task registry#54joelhalen wants to merge 1 commit into
joelhalen wants to merge 1 commit into
Conversation
A tester's client logged "Couldn't fetch the manifest: Expected BEGIN_ARRAY
but was BEGIN_OBJECT at line 1 column 15436 path $.combat_achievement_tasks",
and lost every manifest-driven feature as a result.
The server changed that section from an array of {varbit, boss} to an object
of {varps, tasks} when the task registry started being read from the game
cache. The backend commit noted that no plugin change was needed, which was
true of the data flow -- the client sends raw varps and the server decodes
them at read time -- but not of the manifest contract, and this model was
never updated.
The damage was not confined to the section that changed. Gson's reflective
adapter abandons the whole document at the first field whose type does not
match, so getManifest() caught the exception and returned null, and with it
went the varp list, the quest ids, and the sync cadence and kill switch. The
class documented a guarantee that a bad manifest "can only ever cost us a
feature, never break startup"; that guarantee was implemented as per-field
null checks, which Gson never reaches when the parse fails wholesale.
So Manifest.fromJson() now reads the document into a JsonElement and
deserializes each section independently. A section that will not parse is
logged and dropped on its own. It reads into JsonElement and tests the shape
rather than asking Gson for a JsonObject, because the latter parses whatever
is there and then casts, raising an unchecked ClassCastException past the
caller's catch on an array-shaped document.
The combat_achievement_tasks model goes entirely, rather than being fixed to
match the new shape. The server holds the registry and decodes the raw varps
against it when a profile is read, which means a registry correction applies
retroactively to everything already stored; decoding client-side could only
ever be staler than that. The new payload also carries no varbit ids, so
there is nothing for a client to read directly. StateSnapshot's ca_tasks
field goes with it -- the backend stored that list and never read it back.
Note this is hardening rather than a hotfix. The server now serves only the
sections clients actually read, so the pending 6.0 build parses today's
manifest correctly with no client change; verified by running the model as it
exists at 3c1a518 against the live response. What this buys is that the next
section to change shape costs one feature instead of all of them.
ManifestParseTest covers the wire format, including a payload shaped like the
one that broke, an unreadable section leaving the rest intact, and the Gson
behaviour the section loop exists for -- so if that ever stops being true,
the loop can go with it.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two changes to how the plugin reads
GET /manifest.Parse per section.
Manifest.fromJson()reads the document into aJsonElementand deserializes each section independently, logging and dropping any one section that will not parse. Previously a singlegson.fromJson(body, Manifest.class)meant Gson's reflective adapter abandoned the entire document at the first type mismatch.Remove the
combat_achievement_tasksmodel. The section is deleted rather than reshaped, along withStateSnapshot.ca_tasks.Why
A tester's client logged:
The server changed that section from an array of
{varbit, boss}to an object of{varps, tasks}when the task registry moved to being read from the game cache. The plugin model was never updated.Because Gson gives up on the whole document,
getManifest()returnednulland the client lost every section, not just the broken one — the combat achievement varps (so no CA data synced at all), the quest ids, and the sync cadence and kill switch. The only symptom was one debug line.Removing the task model rather than fixing its shape is deliberate: the server holds the registry and decodes the raw varps against it at read time, so a registry correction applies retroactively to everything already stored. The new payload also carries no varbit ids, so there is nothing for a client to read directly. The backend stored the
ca_taskslist the plugin sent and never read it back.This is hardening, not a hotfix
The server side is already fixed and deployed —
/manifestnow serves only the sections clients actually read (239 bytes, down from 160KB). The 6.0 build pending Plugin Hub approval parses today's manifest correctly with no client change; I verified that by running the model as it exists at3c1a518against the live response. There is no need to pull or resubmit that PR.What this buys is that the next section to change shape costs one feature instead of all of them.
Verification
ManifestParseTest— 6 cases covering the wire format: a payload shaped like the one that broke, an unreadable section leaving the rest intact, missing sections falling back to defaults, and non-object documents returning null rather than throwing./manifestresponse parses cleanly through the new code with no sections dropped.