mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-09-26 01:43:43 +00:00
fix(nodes): keep paginating past a page that post-LIMIT filtering shortened (#2061)
## Problem `handleNodes` runs the geo-filter, `nodeBlacklist`, `hiddenNamePrefixes` and area passes **after** the SQL `LIMIT/OFFSET`, and rewrites `total` to the filtered length. A page that loses a row is therefore short **without being the last page**, and neither the page length nor `total` can tell a client whether to ask for another page. #1606 added the pagination loop and chose the page length as the canonical stop. That is correct only where nothing is ever filtered. Everywhere else the list truncates at the first filtered page boundary and strands every node behind it — the #1598 symptom reached by a different route: a node that is relaying right now simply stops being in the list. The comment at `app.js:240` rejects `total` for exactly the right reason, then picks the signal the same code path also breaks. ## Measured on a live 2346-node deployment Page sizes for the query the map issues: ``` offset=0 returned=500 ← full, loop continues offset=500 returned=499 ← one row filtered AFTER the LIMIT → loop STOPS offset=1000 returned=500 ← never requested offset=1500 returned=500 ← never requested offset=2000 returned=345 ← never requested ``` | stop rule | requests | nodes reached | |---|---:|---:| | short page (master) | 2 | **999** | | `has_more`, else empty page | 6 | **2344** | **1341 nodes, 57%, unreachable through the UI.** ### One hidden node truncates the whole list The deployment this came from has no `geoFilter` (`/api/config/geo-filter` returns `polygon: null`) and no `nodeBlacklist`. It has a single `hiddenNamePrefixes` entry — a deliberate operator choice — and exactly one node whose name starts with it: ``` public_key d4a46ea2…1054 (64 clean hex chars) name 🚫🔥☀️ role repeater last_seen 2026-09-22T10:09:38Z ``` `handleNodes` drops that row in the `IsNameHidden` pass, which runs after the SQL `LIMIT`. The row is counted by the `LIMIT` and by `COUNT(*)`, so the page it lands in comes back exactly one short — and stops every client that treats a short page as the end. Isolated against SQL on the same database, seconds apart: ``` SELECT lower(public_key) FROM nodes ORDER BY last_seen DESC LIMIT 500 OFFSET 500 -> 500 rows GET /api/nodes?limit=500&offset=500 -> 499 rows comm -23 sql.txt api.txt -> d4a46ea2e99cab132a3286ef3d9cce9099318790af7f25671fe83de453721054 ``` Deterministic — `offset=500` returned 499 on three consecutive requests. Not a CDN artifact either: `cf-cache-status: DYNAMIC`, origin `cache-control: no-store`, no `age` header, and four requests with deliberately unique cache keys all returned 499. So **one deliberately hidden node makes 1341 of 2344 nodes unreachable.** The hiding feature does exactly what it was asked to do for that one node, and takes 57% of the network with it, silently. A single `hiddenNamePrefixes` entry is enough; no geo-filter, blacklist or area filter is needed to reach this state. ### The cutoff moves, which is why this reads as intermittent The visible set is the sum of the pages up to and including the first short one, so the boundary sits wherever the unreturnable row currently sorts by `last_seen`, and jumps a whole page as ingest reorders the list. Same deployment, same code, same config, ~2h apart: | dropped row's rank | first short page | nodes visible | |---|---|---:| | inside 0–499 | page 1 | 499 | | inside 500–999 | page 2 | 999 | A node is visible or invisible purely by where it lands relative to that moving line, so affected nodes appear to vanish and return on their own. Two operators on this deployment reported exactly that, independently, while I was measuring. ### A named reproduction `HU-ZA-Lentihegy` (`5287a33f…`), reported missing from the map by an operator whose companion had logged its advert at 04:20 local the same morning. Ingest was fine. The row is in `nodes` with `last_seen` `2026-09-22T02:20:35Z` — the same advert, to the second — valid GPS, role `repeater`, 1033 adverts, and `/api/nodes/search?q=lentihegy` returns it. ``` rank by last_seen : 1081 cutoff at the time: 999 ``` It missed by 82 positions. Walking the same live endpoint, same moment: | stop rule | requests | nodes reached | Lentihegy | |---|---:|---:|---| | short page (master) | 2 | 999 | **not reached** | | `has_more`, else empty page | 6 | 2340 | reached | The practical shape of this on a busy mesh: 1081 nodes had been heard more recently than 9.4 hours, so on that deployment **anything last heard more than ~9 hours ago was invisible**, alive or not. `#/nodes` compounds it — its search box filters client-side over the truncated set, so the server-side `?search=` never runs and an operator cannot find the node by searching for it either, even though the endpoint would return it. ## Change **Server** — `NodeListResponse` gains `has_more`, computed from the raw SQL page against the real `COUNT(*)` before the filter passes run, so it survives them: ```go hasMore := offset+len(nodes) < total ``` Always emitted (no `omitempty`) so a client can tell `false` from an old server. No extra request in the fixed path: `has_more` ends the loop exactly, where the old rule needed a probe page. **Clients** — `app.js` `fetchAllNodes`, `nodes.js` `loadNodes` and `area-map.html`'s inline helper stop on `has_more`, falling back to a zero-length page against a server that predates it. An empty page always ends the loop, so a `has_more` against a concurrently-shrinking table cannot spin to `safetyCap`. Left alone: the three loops are still three copies. Collapsing them onto `fetchAllNodes` is a bigger change than this fix needs, and `nodes.js` has its own inter-page progress UI. Happy to do it separately if you want it. ## Testing - **Unit** (`tests/unit/test-fetch-all-nodes-pagination.js`): the fixture now models the real handler — a row counted by the LIMIT and by `COUNT(*)`, then removed from the page. Three new cases. Fails on the old rule at 499 of 1199. - **E2E** (`tests/e2e/test-map-nodes-pagination-e2e.js`, already wired into `deploy.yml`): the mock drops a page-1 row and emits `has_more`. Mutation-checked — restoring master's stop rule fails 3 of its steps. - **Go** (`cmd/server/nodes_pagination_has_more_test.go`): asserts `has_more` stays true on a page filtering shortened. Mutation-checked — recomputing it after the filter block fails the test. - Full server suite `go test -race`: ok, 41.4s. `gofmt` clean, `go vet` passes. - **Against a real binary**, not just mocks: fixture DB migrated with `corescope-migrate`, `hiddenNamePrefixes: ["SKCE"]`, `limit=3`. Page 1 returns 2 of 3 with `total` rewritten to 2 and `has_more=true`. Walking the real server with master's rule reaches 2 nodes; with `has_more`, all 199 visible of 200, the hidden one still hidden. The real frontend against that server loads 199 with no JS errors. Two existing expectations changed, both deliberate: 1. `surfaces ALL nodes past the 500 server cap` — 3 → 4 requests. That mock emits no `has_more`, so the 200-row final page can no longer end the loop (a short page is exactly what a filtered page looks like) and a zero-length probe follows. Against a current server `has_more` still ends it at 3. 2. `rows missing public_key are NOT collapsed into one` — its stub returned a constant body, which would now be paged to `safetyCap`. It serves one page then empties. Local `test-all.sh` exits 1 on two XSS-gate self-tests (`good-2-tested.js`, `good-4-tested.js`) via a `UnicodeEncodeError` printing an emoji under Windows cp1252. Identical on clean `origin/master` in a scratch worktree, so it is pre-existing and platform-local, not this branch. There is a second identical filter block further down `routes.go` on another list endpoint. Likely the same class; not touched here. If you would rather land your own version of this, say so and I will close mine. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
980c5c4515
commit
b695b979a2
@@ -0,0 +1,92 @@
|
||||
package main
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"fmt"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// TestNodesHasMoreSurvivesPostLimitFiltering pins the contract that lets a
|
||||
// client paginate /api/nodes safely.
|
||||
//
|
||||
// handleNodes applies the geo-filter, blacklist, hidden-prefix and area passes
|
||||
// AFTER the SQL LIMIT/OFFSET, and rewrites Total to the filtered length. So a
|
||||
// page that loses a row is short without being the last page, and neither the
|
||||
// page length nor Total can tell a client whether to ask for another page.
|
||||
// has_more is computed from the raw SQL page against the real COUNT(*), before
|
||||
// those passes run, and must therefore stay true on a page that filtering
|
||||
// shortened.
|
||||
//
|
||||
// Anti-tautology: move the hasMore assignment below the filter block (or
|
||||
// compute it from the filtered slice) and the page-1 assertion fails — that is
|
||||
// exactly the arrangement that stranded every node behind a filtered row.
|
||||
func TestNodesHasMoreSurvivesPostLimitFiltering(t *testing.T) {
|
||||
srv, router := setupTestServer(t)
|
||||
|
||||
// setupTestServer seeds its own fixture nodes; clear them so the page
|
||||
// boundaries below are exactly the ones this test sets up.
|
||||
if _, err := srv.db.conn.Exec(`DELETE FROM nodes`); err != nil {
|
||||
t.Fatalf("clear fixture nodes: %v", err)
|
||||
}
|
||||
|
||||
// 7 nodes, newest first by last_seen so page order is deterministic.
|
||||
for i := 0; i < 7; i++ {
|
||||
name := fmt.Sprintf("visible-%d", i)
|
||||
if i == 1 {
|
||||
name = "🚫 hidden-1" // lands inside page 1 (offset 0, limit 3)
|
||||
}
|
||||
lastSeen := fmt.Sprintf("2026-06-0%dT00:00:00Z", 7-i)
|
||||
if _, err := srv.db.conn.Exec(`INSERT INTO nodes
|
||||
(public_key, name, role, lat, lon, last_seen, first_seen, advert_count)
|
||||
VALUES (?, ?, 'repeater', 0, 0, ?, '2026-06-01T00:00:00Z', 1)`,
|
||||
fmt.Sprintf("deadbeef0000200%d", i), name, lastSeen); err != nil {
|
||||
t.Fatalf("insert %d: %v", i, err)
|
||||
}
|
||||
}
|
||||
srv.cfg.SetHiddenNamePrefixes([]string{"🚫"})
|
||||
|
||||
page := func(offset int) NodeListResponse {
|
||||
req := httptest.NewRequest("GET", fmt.Sprintf("/api/nodes?limit=3&offset=%d", offset), nil)
|
||||
w := httptest.NewRecorder()
|
||||
router.ServeHTTP(w, req)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("offset %d: status %d body=%s", offset, w.Code, w.Body.String())
|
||||
}
|
||||
var got NodeListResponse
|
||||
if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil {
|
||||
t.Fatalf("offset %d: decode: %v", offset, err)
|
||||
}
|
||||
return got
|
||||
}
|
||||
|
||||
// Page 1 is a row short because the hidden node was dropped after the LIMIT.
|
||||
first := page(0)
|
||||
if len(first.Nodes) != 2 {
|
||||
t.Fatalf("page 1: expected 2 rows after filtering, got %d", len(first.Nodes))
|
||||
}
|
||||
if !first.HasMore {
|
||||
t.Fatalf("page 1: has_more must stay true on a page shortened by filtering " +
|
||||
"(4 more nodes are waiting) — a client stopping here strands them")
|
||||
}
|
||||
|
||||
// Walking has_more reaches every visible node, including the last page.
|
||||
seen := map[string]bool{}
|
||||
for offset, more := 0, true; more && offset < 100; offset += 3 {
|
||||
p := page(offset)
|
||||
for _, n := range p.Nodes {
|
||||
pk, _ := n["public_key"].(string)
|
||||
seen[pk] = true
|
||||
}
|
||||
more = p.HasMore
|
||||
}
|
||||
if len(seen) != 6 {
|
||||
t.Fatalf("expected all 6 visible nodes across the walk, got %d", len(seen))
|
||||
}
|
||||
|
||||
// The final page reports has_more=false rather than relying on a short page.
|
||||
if last := page(6); last.HasMore {
|
||||
t.Fatalf("final page: has_more must be false, got true")
|
||||
}
|
||||
}
|
||||
@@ -208,9 +208,10 @@ func componentSchemas() map[string]interface{} {
|
||||
"NodeListResponse": map[string]interface{}{
|
||||
"type": "object",
|
||||
"properties": map[string]interface{}{
|
||||
"nodes": map[string]interface{}{"type": "array", "items": schemaRef("Node")},
|
||||
"total": map[string]interface{}{"type": "integer", "description": "Total nodes matching the query after filtering."},
|
||||
"counts": map[string]interface{}{"type": "object", "additionalProperties": map[string]interface{}{"type": "integer"}, "description": "Per-role node counts."},
|
||||
"nodes": map[string]interface{}{"type": "array", "items": schemaRef("Node")},
|
||||
"total": map[string]interface{}{"type": "integer", "description": "Total nodes matching the query after filtering."},
|
||||
"counts": map[string]interface{}{"type": "object", "additionalProperties": map[string]interface{}{"type": "integer"}, "description": "Per-role node counts."},
|
||||
"has_more": map[string]interface{}{"type": "boolean", "description": "True when rows exist past this page. Decided before the blacklist / hidden-prefix / geo-filter / area passes, which drop rows from the page and rewrite total — so neither the page length nor total can be used to stop paginating. Paginate until this is false."},
|
||||
},
|
||||
},
|
||||
"NodeDetailResponse": map[string]interface{}{
|
||||
|
||||
+12
-3
@@ -1334,9 +1334,10 @@ func (s *Server) handleDecode(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
func (s *Server) handleNodes(w http.ResponseWriter, r *http.Request) {
|
||||
q := r.URL.Query()
|
||||
limit := queryLimit(r, 50, s.cfg.ListLimits.NodesMax)
|
||||
offset := queryInt(r, "offset", 0)
|
||||
nodes, total, counts, err := s.db.GetNodes(
|
||||
queryLimit(r, 50, s.cfg.ListLimits.NodesMax),
|
||||
queryInt(r, "offset", 0),
|
||||
limit, offset,
|
||||
q.Get("role"), q.Get("search"), q.Get("before"),
|
||||
q.Get("lastHeard"), q.Get("sortBy"), q.Get("region"),
|
||||
)
|
||||
@@ -1344,6 +1345,14 @@ func (s *Server) handleNodes(w http.ResponseWriter, r *http.Request) {
|
||||
writeError(w, 500, err.Error())
|
||||
return
|
||||
}
|
||||
// Whether more rows exist is decided HERE, against the raw SQL page and the
|
||||
// real COUNT(*), because both other candidate signals are destroyed further
|
||||
// down: the geo-filter / blacklist / hidden-prefix / area passes drop rows
|
||||
// from this page AND rewrite `total` to the filtered length. A page that
|
||||
// loses a row is then short without being the last page, so a client that
|
||||
// stops on a short page strands every node behind it (#1606 fixed the
|
||||
// no-filter case only). has_more survives those passes untouched.
|
||||
hasMore := offset+len(nodes) < total
|
||||
if s.store != nil {
|
||||
hashInfo := s.store.GetNodeHashSizeInfo()
|
||||
relayWindow := s.cfg.GetHealthThresholds().RelayActiveHours
|
||||
@@ -1513,7 +1522,7 @@ func (s *Server) handleNodes(w http.ResponseWriter, r *http.Request) {
|
||||
total = len(filtered)
|
||||
}
|
||||
}
|
||||
writeJSON(w, NodeListResponse{Nodes: nodes, Total: total, Counts: counts})
|
||||
writeJSON(w, NodeListResponse{Nodes: nodes, Total: total, Counts: counts, HasMore: hasMore})
|
||||
}
|
||||
|
||||
func (s *Server) handleNodeSearch(w http.ResponseWriter, r *http.Request) {
|
||||
|
||||
@@ -435,6 +435,12 @@ type NodeListResponse struct {
|
||||
Nodes []map[string]interface{} `json:"nodes"`
|
||||
Total int `json:"total"`
|
||||
Counts map[string]int `json:"counts"`
|
||||
// HasMore reports whether rows exist past this page. Computed from the raw
|
||||
// SQL page before the post-LIMIT filters in handleNodes, which shorten the
|
||||
// page and rewrite Total — so it is the only field a paginating client can
|
||||
// trust to decide whether to ask for another page. Always emitted (no
|
||||
// omitempty): a client must be able to tell "false" from "old server".
|
||||
HasMore bool `json:"has_more"`
|
||||
}
|
||||
|
||||
type NodeSearchResponse struct {
|
||||
|
||||
+19
-5
@@ -237,11 +237,25 @@ async function fetchAllNodes(extraQuery = '', { ttl = 0, pageSize = 500, safetyC
|
||||
: (Array.isArray(data) ? data : []);
|
||||
accumulated.push.apply(accumulated, page);
|
||||
if (offset === 0) counts = (data && data.counts) || {};
|
||||
// Canonical stop: a short page is the end. The server's `total` is a real
|
||||
// COUNT(*) for the query, but the handler overwrites it with the filtered
|
||||
// length under area/geo/blacklist filtering — so we never loop on it, nor
|
||||
// surface it; a short page is the reliable end-of-data signal. See #1606.
|
||||
if (page.length < pageSize) break;
|
||||
// Canonical stop: the server's `has_more`. Neither of the other two signals
|
||||
// can be trusted — the handler rewrites `total` to the filtered length, AND
|
||||
// the same filters (blacklist / hidden prefix / geo-filter / area) drop rows
|
||||
// from the page itself, so a page can be short while later pages still hold
|
||||
// rows. #1606 stopped on a short page, which is correct only on deployments
|
||||
// where nothing is ever filtered; elsewhere one filtered node in page 1
|
||||
// stranded every node behind it. `has_more` is computed server-side before
|
||||
// those filters run.
|
||||
// An empty page always ends it: there is nothing here, and OFFSET means
|
||||
// nothing behind it either. Checked first so a server reporting has_more
|
||||
// against a concurrently-shrinking table cannot spin us to safetyCap.
|
||||
if (page.length === 0) break;
|
||||
if (data && typeof data.has_more === 'boolean') {
|
||||
if (!data.has_more) break;
|
||||
continue;
|
||||
}
|
||||
// Server predates `has_more`: fall back to a zero-length page, the only
|
||||
// remaining end-of-data signal that filtering cannot fake. Costs one extra
|
||||
// request per load against an old server; a short page no longer stops us.
|
||||
}
|
||||
// Dedup by public_key: the sort window (last_seen DESC by default) can shift
|
||||
// under concurrent ingest, repeating a row across a page boundary. Rows
|
||||
|
||||
@@ -239,7 +239,11 @@ async function fetchAllNodesPaged(extra) {
|
||||
const d = await r.json();
|
||||
const page = Array.isArray(d) ? d : (d.nodes || []);
|
||||
out.push.apply(out, page);
|
||||
if (page.length < PAGE) break;
|
||||
// has_more, not page length: /api/nodes filters rows out AFTER the SQL
|
||||
// LIMIT, so a short page can still have pages behind it. Zero-length is
|
||||
// the fallback for a server that predates the flag.
|
||||
if (page.length === 0) break;
|
||||
if (typeof d.has_more === 'boolean' && !d.has_more) break;
|
||||
}
|
||||
// Dedup by public_key; rows missing one get a unique key so they aren't collapsed.
|
||||
const seen = new Map();
|
||||
|
||||
+14
-5
@@ -1232,9 +1232,10 @@
|
||||
try {
|
||||
// Fetch all nodes via pagination loop — server clamps /api/nodes ?limit
|
||||
// to 500 (PR #1540 / v3.8.3 DoS guard), so a single fetch silently
|
||||
// truncates large deployments. Loop exit uses data.nodes.length < PAGE_SIZE
|
||||
// as canonical stop — server total is unreliable under area filters
|
||||
// (routes.go:1357 overwrites total = len(filtered)). See #1606.
|
||||
// truncates large deployments. Loop exit uses data.has_more — server
|
||||
// total is unreliable under area filters (overwritten with
|
||||
// len(filtered)), and so is the page length, since the same filters drop
|
||||
// rows from the page. See #1606 and the has_more note in app.js.
|
||||
if (!_allNodes) {
|
||||
const PAGE_SIZE = 500;
|
||||
const SAFETY_CAP = 10000; // hard ceiling to bound runaway loops
|
||||
@@ -1263,8 +1264,16 @@
|
||||
const estTotal = firstTotal || '?';
|
||||
nodesBody.innerHTML = '<tr><td colspan="99" style="text-align:center;padding:2em">Loading nodes\u2026 ' + accumulated.length + '/' + estTotal + '</td></tr>';
|
||||
}
|
||||
// M1 fix: exit when page is short (canonical stop), not based on total
|
||||
if (data.nodes.length < PAGE_SIZE) break;
|
||||
// Exit on has_more; fall back to a zero-length page against a server
|
||||
// that predates it. A short page is NOT the end: handleNodes drops
|
||||
// blacklisted / hidden / out-of-geofilter rows after the SQL LIMIT,
|
||||
// so one filtered node in page 1 used to strand the whole rest of
|
||||
// the list — including nodes that are actively relaying right now.
|
||||
// Empty page ends it unconditionally (nothing here, nothing behind
|
||||
// it); otherwise has_more decides, falling back to empty-page on a
|
||||
// server that predates the flag.
|
||||
if (data.nodes.length === 0) break;
|
||||
if (typeof data.has_more === 'boolean' && !data.has_more) break;
|
||||
offset += PAGE_SIZE;
|
||||
}
|
||||
// TODO(m2): per-page cache invalidation — currently each page uses
|
||||
|
||||
@@ -26,6 +26,7 @@ const { chromium } = require('playwright');
|
||||
|
||||
const BASE = process.env.BASE_URL || 'http://localhost:13581';
|
||||
const PAGE_CAP = 500; // client page size; a node sits past it on page 2
|
||||
const FILTERED_INDEX = 450; // a page-1 row the server drops AFTER the SQL LIMIT
|
||||
const PAGE2_KEY = 'page2deadbeef00000000000000000000000000000000000000000000000beef02';
|
||||
const PAGE2_NAME = 'PAGE2 RP';
|
||||
|
||||
@@ -152,13 +153,20 @@ async function checkMapTeardown(browser, stage, revisit) {
|
||||
nodesRequests++;
|
||||
const limit = Math.min(parseInt(url.searchParams.get('limit') || '50', 10), PAGE_CAP);
|
||||
const offset = parseInt(url.searchParams.get('offset') || '0', 10);
|
||||
const slice = fixture.slice(offset, offset + limit);
|
||||
const raw = fixture.slice(offset, offset + limit);
|
||||
// Model handleNodes exactly: the blacklist / hidden-prefix / geo-filter
|
||||
// passes run AFTER the SQL LIMIT, so FILTERED_INDEX is counted by the
|
||||
// limit and by COUNT(*) but removed from the page. Page 1 comes back a
|
||||
// row short WITHOUT being the last page — the shape that stranded every
|
||||
// node behind it once a client stopped on a short page.
|
||||
const slice = raw.filter((_n, i) => offset + i !== FILTERED_INDEX);
|
||||
return route.fulfill({
|
||||
status: 200,
|
||||
contentType: 'application/json',
|
||||
body: JSON.stringify({
|
||||
nodes: slice,
|
||||
total: slice.length, // deliberately wrong per-page total; the helper must ignore it
|
||||
has_more: offset + raw.length < fixture.length, // decided pre-filter, as the server does
|
||||
counts: { repeaters: fixture.length, rooms: 0, companions: 0, sensors: 0 },
|
||||
}),
|
||||
});
|
||||
@@ -173,15 +181,21 @@ async function checkMapTeardown(browser, stage, revisit) {
|
||||
await page.goto(BASE + '/#/map', { waitUntil: 'load', timeout: 60000 });
|
||||
await page.waitForSelector('#leaflet-map', { timeout: 15000 });
|
||||
|
||||
await step('map loads all 501 nodes by paginating past the 500-row cap', async () => {
|
||||
// Wait until loadNodes() has populated the app node set.
|
||||
await step('a page shortened by post-LIMIT filtering does not end pagination', async () => {
|
||||
// 500 = 501 fixture nodes minus the one the mock filters out of page 1.
|
||||
// Pre-fix the run stops on that 499-row page and __mc_nodes holds 499.
|
||||
await page.waitForFunction(
|
||||
() => Array.isArray(window.__mc_nodes) && window.__mc_nodes.length >= 501,
|
||||
() => Array.isArray(window.__mc_nodes) && window.__mc_nodes.length >= 500,
|
||||
{ timeout: 15000 }
|
||||
);
|
||||
const len = await page.evaluate(() => window.__mc_nodes.length);
|
||||
assert(len === 501, 'expected 501 nodes in __mc_nodes, got ' + len);
|
||||
assert(len === 500, 'expected 500 nodes in __mc_nodes (501 minus the filtered row), got ' + len);
|
||||
assert(nodesRequests >= 2, 'expected ≥2 /api/nodes page requests, got ' + nodesRequests);
|
||||
const filteredGone = await page.evaluate(
|
||||
(k) => !window.__mc_nodes.some((n) => n.public_key === k),
|
||||
'p1' + String(FILTERED_INDEX).padStart(62, '0')
|
||||
);
|
||||
assert(filteredGone, 'the filtered row must stay filtered, not reappear');
|
||||
});
|
||||
|
||||
await step('the page-2 node (cut by the cap pre-fix) is present in the node set', async () => {
|
||||
|
||||
@@ -81,6 +81,13 @@ function makeNodesFetch(total, cap, opts = {}) {
|
||||
for (let i = 0; i < total; i++) fixture.push({ public_key: 'pk' + i, name: 'N' + i });
|
||||
// Optionally repeat the last row of page 1 as the first row of page 2.
|
||||
if (opts.dupAtBoundary) fixture[cap] = fixture[cap - 1];
|
||||
// `dropIndexes` models handleNodes' POST-LIMIT filters (geo-filter,
|
||||
// nodeBlacklist, hiddenNamePrefixes, area): the row is counted by the SQL
|
||||
// LIMIT and by COUNT(*), then removed from the page in Go. The page is a row
|
||||
// short WITHOUT being the last page. `hasMore` mirrors the server field,
|
||||
// which is computed before those filters run; omit it to model a server that
|
||||
// predates the field.
|
||||
const dropped = new Set(opts.dropIndexes || []);
|
||||
return {
|
||||
calls,
|
||||
fetch: (url) => {
|
||||
@@ -89,10 +96,13 @@ function makeNodesFetch(total, cap, opts = {}) {
|
||||
const p = new URLSearchParams(qs);
|
||||
const limit = Math.min(parseInt(p.get('limit') || '50', 10), cap);
|
||||
const offset = parseInt(p.get('offset') || '0', 10);
|
||||
const page = fixture.slice(offset, offset + limit);
|
||||
const raw = fixture.slice(offset, offset + limit);
|
||||
const page = raw.filter((_n, i) => !dropped.has(offset + i));
|
||||
const body = { nodes: page, counts: { repeaters: total }, total: page.length };
|
||||
if (opts.hasMore) body.has_more = offset + raw.length < total;
|
||||
return Promise.resolve({
|
||||
ok: true,
|
||||
json: () => Promise.resolve({ nodes: page, counts: { repeaters: total }, total: page.length }),
|
||||
json: () => Promise.resolve(body),
|
||||
});
|
||||
},
|
||||
};
|
||||
@@ -108,13 +118,57 @@ test('surfaces ALL nodes past the 500 server cap (1200 > 500)', async () => {
|
||||
const out = await ctx.fetchAllNodes('');
|
||||
assert.strictEqual(out.nodes.length, 1200, 'expected all 1200 nodes, got ' + out.nodes.length);
|
||||
assert.strictEqual(out.total, 1200, 'total must be the real deduped count, not the clamped per-page total');
|
||||
assert.strictEqual(m.calls.length, 3, 'expected 3 pages (500+500+200), got ' + m.calls.length);
|
||||
// 4, not 3: this mock emits no has_more, so the 200-row page cannot end the
|
||||
// loop (a short page is exactly what a filtered page looks like) and a
|
||||
// zero-length probe follows. Against a current server has_more ends it at 3.
|
||||
assert.strictEqual(m.calls.length, 4, 'expected 3 data pages + 1 probe, got ' + m.calls.length);
|
||||
});
|
||||
|
||||
test('stops on a short page rather than the unreliable server total', async () => {
|
||||
test('a page shortened by a POST-LIMIT filter does NOT end pagination', async () => {
|
||||
const ctx = makeSandbox();
|
||||
loadInCtx(ctx, 'public/app.js');
|
||||
// Exactly 1000 → pages 500, 500, then a 0-length page stops the loop.
|
||||
// One node inside page 1 is dropped by handleNodes' blacklist / hidden-prefix
|
||||
// / geo-filter pass, which runs AFTER the SQL LIMIT. Page 1 returns 499 of
|
||||
// 500 while 700 more rows are waiting. Treating that short page as the end
|
||||
// stranded every node past it (map, Nodes page, live, analytics).
|
||||
const m = makeNodesFetch(1200, 500, { dropIndexes: [450] });
|
||||
ctx.fetch = m.fetch;
|
||||
const out = await ctx.fetchAllNodes('');
|
||||
assert.strictEqual(out.nodes.length, 1199, 'expected 1199 surviving nodes, got ' + out.nodes.length);
|
||||
assert.ok(out.nodes.some(n => n.public_key === 'pk700'), 'a page-2 node must be reachable');
|
||||
assert.ok(!out.nodes.some(n => n.public_key === 'pk450'), 'the filtered node must stay filtered');
|
||||
});
|
||||
|
||||
test('uses the server has_more flag and stops without an extra empty page', async () => {
|
||||
const ctx = makeSandbox();
|
||||
loadInCtx(ctx, 'public/app.js');
|
||||
// Same filtered page, but against a server that reports has_more. Exactly
|
||||
// 1000 rows → two full pages; has_more=false on page 2 ends the loop with no
|
||||
// third request (the short-page fallback needs one to see a zero-length page).
|
||||
const m = makeNodesFetch(1000, 500, { dropIndexes: [10], hasMore: true });
|
||||
ctx.fetch = m.fetch;
|
||||
const out = await ctx.fetchAllNodes('');
|
||||
assert.strictEqual(out.nodes.length, 999, 'expected 999 surviving nodes, got ' + out.nodes.length);
|
||||
assert.strictEqual(m.calls.length, 2, 'has_more must end the loop without a probe page, got ' + m.calls.length);
|
||||
});
|
||||
|
||||
test('has_more=false ends the loop even on a full page', async () => {
|
||||
const ctx = makeSandbox();
|
||||
loadInCtx(ctx, 'public/app.js');
|
||||
// Guards the inverse of the bug: the flag, not the page length, decides.
|
||||
const m = makeNodesFetch(500, 500, { hasMore: true });
|
||||
ctx.fetch = m.fetch;
|
||||
const out = await ctx.fetchAllNodes('');
|
||||
assert.strictEqual(out.nodes.length, 500);
|
||||
assert.strictEqual(m.calls.length, 1, 'a full final page with has_more=false must not be followed, got ' + m.calls.length);
|
||||
});
|
||||
|
||||
test('stops on an EMPTY page when the server predates has_more', async () => {
|
||||
const ctx = makeSandbox();
|
||||
loadInCtx(ctx, 'public/app.js');
|
||||
// Exactly 1000 → pages 500, 500, then a 0-length page stops the loop. Without
|
||||
// has_more a zero-length page is the only trustworthy end-of-data signal, so
|
||||
// the probe request is the documented cost of talking to an older server.
|
||||
const m = makeNodesFetch(1000, 500);
|
||||
ctx.fetch = m.fetch;
|
||||
const out = await ctx.fetchAllNodes('');
|
||||
@@ -171,10 +225,14 @@ test('rows missing public_key are NOT collapsed into one', async () => {
|
||||
const ctx = makeSandbox();
|
||||
loadInCtx(ctx, 'public/app.js');
|
||||
// Two distinct rows both lacking public_key must survive as two entries.
|
||||
ctx.fetch = () => Promise.resolve({
|
||||
ok: true,
|
||||
json: () => Promise.resolve({ nodes: [{ name: 'A' }, { name: 'B' }, { public_key: 'pk1', name: 'C' }] }),
|
||||
});
|
||||
// The stub must stop itself: with the short-page rule gone, a constant body
|
||||
// would be paged until safetyCap. One data page, then empty.
|
||||
let served = false;
|
||||
ctx.fetch = () => {
|
||||
const nodes = served ? [] : [{ name: 'A' }, { name: 'B' }, { public_key: 'pk1', name: 'C' }];
|
||||
served = true;
|
||||
return Promise.resolve({ ok: true, json: () => Promise.resolve({ nodes }) });
|
||||
};
|
||||
const out = await ctx.fetchAllNodes('');
|
||||
assert.strictEqual(out.nodes.length, 3, 'falsy-key rows must not collapse, got ' + out.nodes.length);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user