fix(federation): removed RebaseError::InterfaceObjectTypename error - #8109
Conversation
…unnecessary - added a regression test that caused the error previously
This comment has been minimized.
This comment has been minimized.
✅ Docs preview readyThe preview is ready to be viewed. View the preview File Changes 0 new, 1 changed, 0 removedBuild ID: 1c21cb2641fbdaed17b06fde URL: https://www.apollographql.com/docs/deploy-preview/1c21cb2641fbdaed17b06fde |
sachindshinde
left a comment
There was a problem hiding this comment.
This is fine given our current limitations of interface object handling; for folks who may not know why we need to do this, see this comment from a previous PR.
Note that this change cannot be mirrored to the JS codebase as-is, as the JS codebase still implements the fragment-reuse optimization which uses errorIfCannotRebase: false, and that codepath needs __typename to be "skipped" to prevent the issue described in this federation repo PR. This fragment-reuse codepath was removed from the Rust codebase, so it's fine here.
Mines the router CHANGELOG and FED-* tickets in apollo-federation source comments for bug fixes the harness would not catch today. Each row is a real shipped planner bug, the directive/feature surface needed to reproduce it, and the corresponding gap in our generator. Notable: FED-505 (missing ConditionNode for @skip/@include), PR #8016 (@requires subgraph-jump bug), PR #7929 (progressive @OverRide on interface implementations), PR #8109 (@interfaceObject __typename handling), PR #7580 (renamed root types in @defer), PR #9123/#9250 (defer dependencies after transitive reduction). Suggests a priority-ordered backlog. Top two are cheap and high-payoff: operation variables + @skip/@include (FED-505 territory) and renamed root types (PR #7580 territory).
Swept three intermediate apollo-federation versions against the in-tree HEAD (2.13.1) to locate when each known divergence pattern was fixed. Results (1000 ops/seed, default seed=17 unless noted): =2.13.0 -> 2.13.1 0 divergences =2.5.0 -> 2.13.1 (seeds 17,42,1234,99) 0 divergences across 4000 ops =2.1.3 -> 2.13.1 87 divergences =2.0.0 -> 2.13.1 87 - 110 divergences (Phases I, J) So PR #7580 (extraneous `... on Query` inline fragment) and FED-505 (missing `Condition` plan node for `@skip`/`@include`) were both fixed in the **(2.1.3, 2.5.0]** window. The 2.5.x → 2.13.x window is converged on consistent plan output across this generator's covered surface. Implication for finding *new* bugs: more chances at the same generator will not surface new patterns on this version pair. Likely high-value next moves are listed in the new READMEs: - @interfaceObject (PR #8109 territory) - @provides (different planner code path than @requires) - richer operation generator (deeper fragments, named definitions) - semantic comparison (compare *responses* not just plans) Artifacts: - tests/regressions/cross_version_2.1.3/README.md + curated diff - tests/regressions/cross_version_2.13.0/README.md (zero-divergence baseline kept as a regression sentinel for future patch releases)
Summary
rebase_onmethod was prohibiting __typename field from being rebased on an interface object type by raisingInterfaceObjectTypenameerror.__typenamefrom interface object types, there are already several other cases where such__typenameselections are added to interface object types.__typenameselections are benign and we currently don't plan to remove all of those cases at this time.Checklist
Complete the checklist (and note appropriate exceptions) before the PR is marked ready-for-review.