Skip to content

Commit 0f8ba88

Browse files
committed
Simplify benchmark review helpers
Use stable fixture key pairs without a mutable cache, share error message formatting through the CLI utilities, pass suite defaults to unsafe override planning directly, and fold DNS tier fallback handling into a single conservative catch path. #795 (comment) #795 (comment) #795 (comment) #795 (comment) Assisted-by: Codex:gpt-5.5
1 parent 2bfa70d commit 0f8ba88

6 files changed

Lines changed: 36 additions & 40 deletions

File tree

packages/cli/src/bench/action.ts

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { writeFile } from "node:fs/promises";
22
import type { DocumentLoader } from "@fedify/vocab-runtime";
33
import process from "node:process";
44
import { getContextLoader, getDocumentLoader } from "../docloader.ts";
5+
import { describeError } from "../utils.ts";
56
import { buildFleet } from "./actor/fleet.ts";
67
import type { BenchCommand } from "./command.ts";
78
import {
@@ -24,7 +25,7 @@ import {
2425
type ResolvedScenario,
2526
type ResolvedSuite,
2627
} from "./scenario/normalize.ts";
27-
import type { LoadConfig, Suite } from "./scenario/types.ts";
28+
import type { LoadConfig, Suite, SuiteDefaults } from "./scenario/types.ts";
2829
import { validateSuite } from "./scenario/validate.ts";
2930
import {
3031
assertInboxDestinationAllowed,
@@ -100,7 +101,7 @@ export default async function runBench(
100101
validated = validateSuite(rendered, command.scenario);
101102
suite = normalizeSuite(validated, { target: command.target });
102103
} catch (error) {
103-
log(error instanceof Error ? error.message : String(error));
104+
log(describeError(error));
104105
return void exit(2);
105106
}
106107

@@ -119,7 +120,7 @@ export default async function runBench(
119120
resolveAdvertiseHost(command.advertiseHost);
120121
}
121122
} catch (error) {
122-
log(error instanceof Error ? error.message : String(error));
123+
log(describeError(error));
123124
return void exit(2);
124125
}
125126

@@ -190,7 +191,7 @@ export default async function runBench(
190191
allowUnsafe: command.allowUnsafeTarget,
191192
explicitCliTarget: command.target != null,
192193
destinationTier,
193-
suite: validated,
194+
defaults: validated.defaults,
194195
});
195196
};
196197

@@ -207,7 +208,7 @@ export default async function runBench(
207208
);
208209
return void exit(0);
209210
} catch (error) {
210-
log(error instanceof Error ? error.message : String(error));
211+
log(describeError(error));
211212
return void exit(2);
212213
}
213214
}
@@ -431,10 +432,6 @@ async function describeInboxDiscoveryPlan(
431432
return lines;
432433
}
433434

434-
function describeError(error: unknown): string {
435-
return error instanceof Error ? error.message : String(error);
436-
}
437-
438435
function describeWebFingerPlan(
439436
scenario: ResolvedScenario,
440437
target: URL,
@@ -472,7 +469,7 @@ interface PublicDestinationOverrideContext {
472469
readonly allowUnsafe: boolean;
473470
readonly explicitCliTarget: boolean;
474471
readonly destinationTier: TargetTier;
475-
readonly suite: Suite;
472+
readonly defaults?: SuiteDefaults;
476473
}
477474

478475
function assertPublicDestinationOverrideAllowed(
@@ -493,24 +490,24 @@ function assertPublicDestinationOverrideAllowed(
493490
benchmarkMode: false,
494491
allowUnsafe: true,
495492
explicitCliTarget: context.explicitCliTarget,
496-
scenarios: [unsafeOverrideScenario(scenario, context.suite)],
493+
scenarios: [unsafeOverrideScenario(scenario, context.defaults)],
497494
});
498495
}
499496

500497
function unsafeOverrideScenarios(
501498
suite: Suite,
502499
): Parameters<typeof assertUnsafeOverrideAllowed>[0]["scenarios"] {
503500
return suite.scenarios.map((scenario) =>
504-
unsafeOverrideScenario(scenario, suite)
501+
unsafeOverrideScenario(scenario, suite.defaults)
505502
);
506503
}
507504

508505
function unsafeOverrideScenario(
509506
scenario: ResolvedScenario | Suite["scenarios"][number],
510-
suite?: Suite,
507+
defaults?: SuiteDefaults,
511508
): Parameters<typeof assertUnsafeOverrideAllowed>[0]["scenarios"][number] {
512-
const defaultDuration = suite?.defaults?.duration != null;
513-
const defaultLoad = hasExplicitLoad(suite?.defaults?.load);
509+
const defaultDuration = defaults?.duration != null;
510+
const defaultLoad = hasExplicitLoad(defaults?.load);
514511
const raw = "raw" in scenario ? scenario.raw : scenario;
515512
return {
516513
name: scenario.name,

packages/cli/src/bench/safety/tiers.ts

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -56,26 +56,21 @@ export async function classifyResolvedTarget(
5656
const host = normalizedHost(target);
5757
const direct = classifyTarget(target);
5858
if (direct !== "public" || isIpLiteral(host)) return direct;
59-
let addresses: readonly string[];
6059
try {
6160
const resolved = await resolveAddresses(host);
62-
addresses = Array.isArray(resolved) ? resolved : [];
61+
if (!Array.isArray(resolved) || resolved.length < 1) return "public";
62+
let aggregate: TargetTier = "loopback";
63+
for (const address of resolved) {
64+
const tier = classifyTarget(
65+
new URL(`http://${hostForAddress(address)}/`),
66+
);
67+
if (tier === "public") return "public";
68+
if (tier === "private") aggregate = "private";
69+
}
70+
return aggregate;
6371
} catch {
6472
return "public";
6573
}
66-
if (addresses.length < 1) return "public";
67-
let aggregate: TargetTier = "loopback";
68-
for (const address of addresses) {
69-
let tier: TargetTier;
70-
try {
71-
tier = classifyTarget(new URL(`http://${hostForAddress(address)}/`));
72-
} catch {
73-
return "public";
74-
}
75-
if (tier === "public") return "public";
76-
if (tier === "private") aggregate = "private";
77-
}
78-
return aggregate;
7974
}
8075

8176
/** Resolves a hostname with the platform DNS resolver. */

packages/cli/src/lookup.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ import {
5959
userAgentOption,
6060
} from "./options.ts";
6161
import { spawnTemporaryServer, type TemporaryServer } from "./tempserver.ts";
62-
import { colorEnabled, colors, formatObject } from "./utils.ts";
62+
import { colorEnabled, colors, describeError, formatObject } from "./utils.ts";
6363

6464
const logger = getLogger(["fedify", "cli", "lookup"]);
6565

@@ -538,7 +538,7 @@ function handleTimeoutError(
538538
}
539539

540540
function isPrivateAddressError(error: unknown): boolean {
541-
const errorMessage = error instanceof Error ? error.message : String(error);
541+
const errorMessage = describeError(error);
542542
const lowerMessage = errorMessage.toLowerCase();
543543
if (error instanceof UrlError) {
544544
return (
@@ -600,7 +600,7 @@ function getPrivateContextUrl(error: unknown): URL | null {
600600
// a trailing `URL: "..."` segment all at once. If jsonld changes those
601601
// details, this helper and the related lookup tests need to be updated
602602
// together.
603-
const errorMessage = error instanceof Error ? error.message : String(error);
603+
const errorMessage = describeError(error);
604604
if (
605605
!(error instanceof Error) ||
606606
error.name !== "jsonld.InvalidUrl" ||

packages/cli/src/runner.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { nodeInfoCommand } from "./nodeinfo.ts";
1616
import { globalOptions } from "./options.ts";
1717
import { relayCommand } from "./relay/command.ts";
1818
import { tunnelCommand } from "./tunnel.ts";
19+
import { describeError } from "./utils.ts";
1920
import { webFingerCommand } from "./webfinger/mod.ts";
2021
import metadata from "../deno.json" with { type: "json" };
2122

@@ -105,7 +106,7 @@ export function loadConfig(
105106
} catch (error) {
106107
printError(
107108
message`Could not load config file at ${parsed.configPath}: ${
108-
error instanceof Error ? error.message : String(error)
109+
describeError(error)
109110
}`,
110111
);
111112
process.exit(1);

packages/cli/src/utils.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,10 @@ export const isNotFoundError = (e: unknown): e is { code: "ENOENT" } =>
8181
"code" in e &&
8282
e.code === "ENOENT";
8383

84+
export function describeError(error: unknown): string {
85+
return error instanceof Error ? error.message : String(error);
86+
}
87+
8488
export class CommandError extends Error {
8589
public commandLine: string;
8690
constructor(

packages/cli/test/bench/fixture.ts

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,10 @@ export async function spawnBenchmarkTarget(): Promise<BenchmarkTargetFixture> {
3838
kv: new MemoryKvStore(),
3939
benchmarkMode: true,
4040
});
41-
let keyPairs: CryptoKeyPair[] | undefined;
41+
const keyPairs = Promise.all([
42+
generateCryptoKeyPair("RSASSA-PKCS1-v1_5"),
43+
generateCryptoKeyPair("Ed25519"),
44+
]);
4245
const requests: { method: string; path: string }[] = [];
4346
federation
4447
.setActorDispatcher("/users/{identifier}", async (ctx, identifier) => {
@@ -56,11 +59,7 @@ export async function spawnBenchmarkTarget(): Promise<BenchmarkTargetFixture> {
5659
.mapHandle((_ctx, username) => (username === "alice" ? "alice" : null))
5760
.setKeyPairsDispatcher(async (_ctx, identifier) => {
5861
if (identifier !== "alice") return [];
59-
keyPairs ??= [
60-
await generateCryptoKeyPair("RSASSA-PKCS1-v1_5"),
61-
await generateCryptoKeyPair("Ed25519"),
62-
];
63-
return keyPairs;
62+
return await keyPairs;
6463
});
6564
federation.setInboxListeners("/users/{identifier}/inbox", "/inbox").on(
6665
Create,

0 commit comments

Comments
 (0)