Skip to content

Fail fast when the concurrent PATCH route is not ready #2204

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
kriszyp wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Fail fast when the concurrent PATCH route is not ready #2204

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 18 additions & 6 deletions integrationTests/database/concurrentPatchMerge.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,20 +62,32 @@ suite(`QA-328 concurrent PATCH field-level merge — ${ENGINE}`, (ctx: ContextWi
httpURL = ctx.harper.httpURL;
auth = client.headers.Authorization;

// Readiness poll — wait until CollabDoc endpoint responds (not 404)
const deadline = Date.now() + 30_000;
const readinessURL = `${httpURL}/CollabDoc/`;
const readinessDeadlineMs = 30_000;
const deadline = Date.now() + readinessDeadlineMs;
let ready = false;
let lastObserved = 'no response';
while (Date.now() < deadline) {
try {
const probe = await fetch(`${httpURL}/CollabDoc/`, {
const probe = await fetch(readinessURL, {
headers: { Authorization: auth },
signal: AbortSignal.timeout(3_000),
});
if (probe.status !== 404) break;
} catch {
/* not ready yet */
lastObserved = `HTTP ${probe.status}`;
await probe.body?.cancel();
if (probe.ok) {
ready = true;
break;
}
} catch (error) {
lastObserved = error instanceof Error ? `${error.name}: ${error.message}` : String(error);
}
await sleep(250);
}
if (!ready)
throw new Error(
`Harper did not become ready at ${readinessURL} within ${readinessDeadlineMs}ms; last observed ${lastObserved}`
);
Comment on lines +65 to +90

@claude claude Bot Aug 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion (non-blocking): This re-implements the same poll/timeout/last-observed-error pattern as waitForRouteReady(client, probePath, timeoutMs) in integrationTests/apiTests/utils/lifecycle.mjs (extracted the same day in #1904), and client here (from createApiClient) already exposes the compatible reqRest() method. Reusing it would drop ~20 duplicated lines. The one real difference is the readiness predicate — the helper treats any non-404 as ready, while this test requires a true 2xx (probe.ok) — which the PR body already flags as an open decision. If that stricter predicate is needed here, consider adding an optional predicate parameter to the shared helper instead of a second bespoke implementation with different semantics.

});

after(async () => {
Expand Down
Loading