Skip to content

Question: is Site::navigation's silent fallback to root for unknown section_ref intentional? #405

Description

@yumike

Question

Site::navigation(Some(\"domain:default/typo\")) — where the section ref doesn't match any section — currently returns the root navigation with scope: Some(root_scope_info), indistinguishable from Site::navigation(None) at the HTTP/JSON layer (other than the optional scope.section field, which a client would have to compare against its request to detect the mismatch).

Internally, SiteState::navigation(\"nonexistent-path\") does the opposite — it returns Navigation::default() (empty items, scope: None, parent_scope: None). The Site wrapper specifically pre-collapses unknown refs to \"\" before calling SiteState::navigation, which is what produces the root-fallback behavior.

I'd like to confirm whether the current behavior is intentional or worth changing. Both interpretations have a case.

Where the behavior lives

crates/rw-site/src/site.rs:199-207:

pub fn navigation(&self, section_ref: Option<&str>) -> Result<Navigation, StorageError> {
    let snapshot = self.reload_if_needed()?;
    let scope_path = section_ref
        .and_then(|r| snapshot.sections.find_by_ref(r).map(str::to_owned))
        .unwrap_or_default();                       // <-- unknown ref → \"\"
    let mut nav = snapshot.state.navigation(&scope_path);
    nav.apply_sections(&snapshot.sections);
    Ok(nav)
}

crates/rw-site/src/site_state.rs:328-371:

pub fn navigation(&self, scope_path: &str) -> Navigation {
    if scope_path.is_empty() {
        // Root scope — items + Some(root_scope_info)
        ...
    } else {
        let Some(section) = self.sections.get(scope_path) else {
            return Navigation::default();           // <-- unknown path → empty
        };
        ...
    }
}

Both HTTP and napi consumers forward the result verbatim:

  • rw-server/src/handlers/navigation.rs:94: state.site.navigation(query.section_ref.as_deref())? → 200 OK with the response.
  • rw-napi/src/lib.rs:195: same.

When this surfaces

Two realistic flows where a section_ref becomes stale or wrong:

  1. Backstage embed (mountRw) — the host application passes sectionRef derived from an entity ref. If the entity is renamed, archived, or the docs tree is reorganized, the host's cached/stored ref no longer matches. The docs panel quietly renders root navigation under what looks like the original ref. From the user's perspective: "the section's docs disappeared."
  2. Bookmark / shared URL?section_ref=... in a URL becomes stale after a section is renamed. The browser silently serves root nav.

Both are minor (no crash, no corruption), but the failure is invisible: the response status is 200 and the only signal is that scope.section doesn't match what was asked for — which clients are not currently required (or documented) to verify.

Possible interpretations

"It's intentional — graceful degradation"

  • Bad/missing refs shouldn't break the page; falling back to root is friendlier than a 404.
  • Avoids edge cases during live reloads where a section briefly disappears between scans.
  • Existing rw-server / rw-napi callers rely on this not erroring during dynamic section reconfigurations.

If this is the case, it's probably worth documenting on Site::navigation so the contract is explicit. Maybe also echoing the requested section_ref in the response (e.g., requestedScope: Option<...>) so a client that does care can detect the fallback without comparing the section object.

"It's an inconsistency — should match SiteState's behavior"

  • SiteState::navigation returns empty nav with scope: None for unknown paths; Site::navigation should do the same for unknown refs.
  • Wrong content under a 200 OK is a class of bug that's hard to catch in monitoring.
  • A SectionNotFound variant on the return type (or an explicit Option<Navigation> for the section-ref case) would make the missing-ref state visible to callers.

If this is the call, a minimal change is to skip the unwrap_or_default() and instead branch:

let scope_path = match section_ref {
    None => String::new(),
    Some(r) => match snapshot.sections.find_by_ref(r) {
        Some(p) => p.to_owned(),
        None => return Ok(Navigation::default()),  // or a typed error
    },
};

That would surface unknown refs as empty navigation at the API layer (HTTP would return 200 with { items: [], scope: null, parentScope: null }), matching the internal SiteState behavior and giving callers an unambiguous signal.

What I'd like to know

  1. Was the current behavior chosen deliberately (graceful degradation), or is it an artifact of unwrap_or_default() being the convenient default?
  2. If intentional, would a docstring note + a requestedScope echo on the response be welcome?
  3. If unintentional, would the minimal patch above be the right direction, or do you prefer a typed error variant (StorageError is the current return type, which isn't really the right home for "section not found")?

Happy to send a PR once the direction is clear. No urgency — this isn't a crash or data-corruption bug, just an observability/contract question.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions