fix(FN-2510): harden skills catalog fallback for short and invalid queries
- Update skills adapter fallback logic to handle empty and short catalog queries without surfacing 400 failures - Handle ApiRequestError paths in SkillsView so invalid-query responses degrade gracefully in the UI - Add regression coverage for adapter, routes, and SkillsView behavior across empty/short query cases - Document short-query catalog behavior in the dashboard guide and include a patch changeset for @runfusion/fusion
This commit is contained in:
@@ -63,15 +63,34 @@ export function SkillsView({ projectId, addToast, onClose }: SkillsViewProps) {
|
||||
|
||||
// Fetch catalog
|
||||
const loadCatalog = useCallback(async (query: string) => {
|
||||
const isCatalogUnavailableError = (error: unknown): boolean => {
|
||||
if (!error || typeof error !== "object") {
|
||||
return false;
|
||||
}
|
||||
|
||||
const withStatus = error as { status?: unknown; details?: unknown };
|
||||
if (typeof withStatus.status === "number" && withStatus.status >= 500) {
|
||||
return true;
|
||||
}
|
||||
|
||||
if (withStatus.details && typeof withStatus.details === "object") {
|
||||
const details = withStatus.details as { code?: unknown };
|
||||
if (typeof details.code === "string" && details.code.startsWith("upstream_")) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
const legacy = error as { error?: unknown; code?: unknown };
|
||||
return typeof legacy.error === "string" && typeof legacy.code === "string";
|
||||
};
|
||||
|
||||
setIsLoadingCatalog(true);
|
||||
setCatalogError(null);
|
||||
try {
|
||||
const result = await fetchSkillsCatalog(query, 20, projectId);
|
||||
setCatalogEntries(result.entries);
|
||||
} catch (err) {
|
||||
// Check for upstream error with code (502 etc.)
|
||||
const error = err as { error?: string; code?: string };
|
||||
if (error.error && error.code) {
|
||||
if (isCatalogUnavailableError(err)) {
|
||||
setCatalogError("Catalog is temporarily unavailable. Please try again later.");
|
||||
} else {
|
||||
const message = err instanceof Error ? err.message : "Failed to load catalog";
|
||||
|
||||
@@ -336,11 +336,13 @@ describe("SkillsView", () => {
|
||||
});
|
||||
|
||||
describe("error handling", () => {
|
||||
it("shows error message for catalog fetch with upstream error", async () => {
|
||||
mockFetchSkillsCatalog.mockRejectedValue({
|
||||
error: "Service unavailable",
|
||||
code: "upstream_http_error",
|
||||
});
|
||||
it("shows friendly error message for catalog fetch with upstream ApiRequestError", async () => {
|
||||
mockFetchSkillsCatalog.mockRejectedValue(
|
||||
Object.assign(new Error("Upstream returned 400: Bad Request"), {
|
||||
status: 502,
|
||||
details: { code: "upstream_http_error" },
|
||||
})
|
||||
);
|
||||
|
||||
render(<SkillsView addToast={mockAddToast} onClose={onClose} />);
|
||||
|
||||
@@ -363,10 +365,12 @@ describe("SkillsView", () => {
|
||||
});
|
||||
|
||||
it("shows Try Again button for catalog error", async () => {
|
||||
mockFetchSkillsCatalog.mockRejectedValue({
|
||||
error: "Service unavailable",
|
||||
code: "upstream_http_error",
|
||||
});
|
||||
mockFetchSkillsCatalog.mockRejectedValue(
|
||||
Object.assign(new Error("Upstream returned 400: Bad Request"), {
|
||||
status: 502,
|
||||
details: { code: "upstream_http_error" },
|
||||
})
|
||||
);
|
||||
|
||||
render(<SkillsView addToast={mockAddToast} onClose={onClose} />);
|
||||
|
||||
@@ -569,10 +573,12 @@ describe("SkillsView", () => {
|
||||
|
||||
describe("error-state retry", () => {
|
||||
it("retry button is displayed when catalog fetch fails", async () => {
|
||||
mockFetchSkillsCatalog.mockRejectedValue({
|
||||
error: "Service unavailable",
|
||||
code: "upstream_http_error",
|
||||
});
|
||||
mockFetchSkillsCatalog.mockRejectedValue(
|
||||
Object.assign(new Error("Upstream returned 400: Bad Request"), {
|
||||
status: 502,
|
||||
details: { code: "upstream_http_error" },
|
||||
})
|
||||
);
|
||||
|
||||
render(<SkillsView addToast={mockAddToast} onClose={onClose} />);
|
||||
|
||||
|
||||
@@ -531,7 +531,7 @@ describe("Skills routes", () => {
|
||||
expect(mockAdapter.fetchCatalog).toHaveBeenCalledWith({ limit: 20, query: "react" });
|
||||
});
|
||||
|
||||
it("returns results when query is omitted (adapter handles wildcard fallback)", async () => {
|
||||
it("returns results when query is omitted (adapter handles default catalog behavior)", async () => {
|
||||
const mockAdapter = createMockSkillsAdapter({
|
||||
fetchCatalog: vi.fn().mockResolvedValue({
|
||||
entries: [
|
||||
|
||||
@@ -215,6 +215,106 @@ describe("createSkillsAdapter - fetchCatalog fallback behavior", () => {
|
||||
expect(result.auth.fallbackUsed).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
it.each([undefined, "", "a"]) (
|
||||
"returns empty success result without upstream call when unauthenticated query is short (%s)",
|
||||
async (query) => {
|
||||
delete process.env.SKILLS_SH_TOKEN;
|
||||
|
||||
globalThis.fetch = vi.fn();
|
||||
|
||||
const adapter = createSkillsAdapter({
|
||||
packageManager: { resolve: vi.fn().mockResolvedValue({ skills: [] }) },
|
||||
getSettingsPath: vi.fn().mockReturnValue("/tmp/settings.json"),
|
||||
});
|
||||
|
||||
const result = await adapter.fetchCatalog({ limit: 20, query });
|
||||
|
||||
expect(globalThis.fetch).not.toHaveBeenCalled();
|
||||
expect("entries" in result).toBe(true);
|
||||
if ("entries" in result) {
|
||||
expect(result.entries).toEqual([]);
|
||||
expect(result.auth).toEqual({
|
||||
mode: "unauthenticated",
|
||||
tokenPresent: false,
|
||||
fallbackUsed: false,
|
||||
});
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it.each([400, 401, 403])(
|
||||
"returns empty success fallback when auth request fails with %i and query is short",
|
||||
async (status) => {
|
||||
process.env.SKILLS_SH_TOKEN = "test-token";
|
||||
|
||||
globalThis.fetch = vi.fn().mockResolvedValue({
|
||||
ok: false,
|
||||
status,
|
||||
statusText: "Bad Request",
|
||||
json: () => Promise.resolve(null),
|
||||
}) as unknown as typeof fetch;
|
||||
|
||||
const adapter = createSkillsAdapter({
|
||||
packageManager: { resolve: vi.fn().mockResolvedValue({ skills: [] }) },
|
||||
getSettingsPath: vi.fn().mockReturnValue("/tmp/settings.json"),
|
||||
});
|
||||
|
||||
const result = await adapter.fetchCatalog({ limit: 20, query: "a" });
|
||||
|
||||
expect(globalThis.fetch).toHaveBeenCalledTimes(1);
|
||||
expect("entries" in result).toBe(true);
|
||||
if ("entries" in result) {
|
||||
expect(result.entries).toEqual([]);
|
||||
expect(result.auth).toEqual({
|
||||
mode: "fallback-unauthenticated",
|
||||
tokenPresent: true,
|
||||
fallbackUsed: true,
|
||||
});
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it("keeps fallback public search behavior for valid queries", async () => {
|
||||
process.env.SKILLS_SH_TOKEN = "test-token";
|
||||
|
||||
globalThis.fetch = vi.fn().mockImplementation((url: string | URL | Request) => {
|
||||
const urlStr = typeof url === "string" ? url : url.toString();
|
||||
if (urlStr.includes("/api/v1/skills")) {
|
||||
return Promise.resolve({
|
||||
ok: false,
|
||||
status: 401,
|
||||
statusText: "Unauthorized",
|
||||
json: () => Promise.resolve(null),
|
||||
});
|
||||
}
|
||||
|
||||
expect(urlStr).toContain("/api/search");
|
||||
expect(urlStr).toContain("q=react");
|
||||
return Promise.resolve({
|
||||
ok: true,
|
||||
status: 200,
|
||||
headers: new Map([["content-type", "application/json"]]),
|
||||
json: () => Promise.resolve({ skills: [{ id: "search-1", name: "React Skill" }] }),
|
||||
});
|
||||
}) as unknown as typeof fetch;
|
||||
|
||||
const adapter = createSkillsAdapter({
|
||||
packageManager: { resolve: vi.fn().mockResolvedValue({ skills: [] }) },
|
||||
getSettingsPath: vi.fn().mockReturnValue("/tmp/settings.json"),
|
||||
});
|
||||
|
||||
const result = await adapter.fetchCatalog({ limit: 20, query: "react" });
|
||||
|
||||
expect(globalThis.fetch).toHaveBeenCalledTimes(2);
|
||||
expect("entries" in result).toBe(true);
|
||||
if ("entries" in result) {
|
||||
expect(result.entries).toHaveLength(1);
|
||||
expect(result.entries[0]!.name).toBe("React Skill");
|
||||
expect(result.auth.mode).toBe("fallback-unauthenticated");
|
||||
expect(result.auth.fallbackUsed).toBe(true);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("createSkillsAdapter - readSkillContent", () => {
|
||||
|
||||
@@ -418,11 +418,20 @@ export function createSkillsAdapter(options: {
|
||||
// Get skills.sh token if available
|
||||
const token = process.env.SKILLS_SH_TOKEN;
|
||||
const catalogUrl = buildCatalogUrl(boundedLimit, query);
|
||||
const searchUrl = buildSearchUrl(boundedLimit, query);
|
||||
const publicSearchQuery = getPublicSearchQuery(query);
|
||||
|
||||
// No token available - use public search endpoint
|
||||
// No token available - use public search endpoint for valid search queries only.
|
||||
// Empty/short queries return a deterministic empty catalog result.
|
||||
if (!token) {
|
||||
return fetchPublicCatalog(searchUrl, {
|
||||
if (!publicSearchQuery) {
|
||||
return buildEmptyCatalogResult({
|
||||
mode: "unauthenticated",
|
||||
tokenPresent: false,
|
||||
fallbackUsed: false,
|
||||
});
|
||||
}
|
||||
|
||||
return fetchPublicCatalog(boundedLimit, publicSearchQuery, {
|
||||
mode: "unauthenticated",
|
||||
tokenPresent: false,
|
||||
fallbackUsed: false,
|
||||
@@ -453,7 +462,15 @@ export function createSkillsAdapter(options: {
|
||||
|
||||
// 400/401/403 from authenticated request - fall back to public search endpoint
|
||||
if (authResponse.status === 400 || authResponse.status === 401 || authResponse.status === 403) {
|
||||
return fetchPublicCatalog(searchUrl, {
|
||||
if (!publicSearchQuery) {
|
||||
return buildEmptyCatalogResult({
|
||||
mode: "fallback-unauthenticated",
|
||||
tokenPresent: true,
|
||||
fallbackUsed: true,
|
||||
});
|
||||
}
|
||||
|
||||
return fetchPublicCatalog(boundedLimit, publicSearchQuery, {
|
||||
mode: "fallback-unauthenticated",
|
||||
tokenPresent: true,
|
||||
fallbackUsed: true,
|
||||
@@ -561,14 +578,11 @@ function buildCatalogUrl(limit: number, query?: string): string {
|
||||
/**
|
||||
* Build the public search URL.
|
||||
*
|
||||
* The search API requires a non-empty query. For initial catalog browsing,
|
||||
* use "*" as a wildcard query.
|
||||
* The search API requires a non-empty query that meets minimum length.
|
||||
*/
|
||||
function buildSearchUrl(limit: number, query?: string): string {
|
||||
function buildSearchUrl(limit: number, query: string): string {
|
||||
const params = new URLSearchParams();
|
||||
const normalizedQuery = query?.trim();
|
||||
|
||||
params.set("q", normalizedQuery && normalizedQuery.length > 0 ? normalizedQuery : "*");
|
||||
params.set("q", query);
|
||||
params.set("limit", String(limit));
|
||||
|
||||
return `https://skills.sh/api/search?${params.toString()}`;
|
||||
@@ -577,10 +591,27 @@ function buildSearchUrl(limit: number, query?: string): string {
|
||||
/**
|
||||
* Fetch and normalize catalog data from the public /api/search endpoint.
|
||||
*/
|
||||
const MIN_PUBLIC_SEARCH_QUERY_LENGTH = 2;
|
||||
|
||||
function getPublicSearchQuery(query?: string): string | null {
|
||||
const normalized = query?.trim() ?? "";
|
||||
return normalized.length >= MIN_PUBLIC_SEARCH_QUERY_LENGTH ? normalized : null;
|
||||
}
|
||||
|
||||
function buildEmptyCatalogResult(auth: CatalogFetchResult["auth"]): CatalogFetchResult {
|
||||
return {
|
||||
entries: [],
|
||||
auth,
|
||||
};
|
||||
}
|
||||
|
||||
async function fetchPublicCatalog(
|
||||
url: string,
|
||||
limit: number,
|
||||
query: string,
|
||||
auth: CatalogFetchResult["auth"],
|
||||
): Promise<CatalogFetchResult | UpstreamError> {
|
||||
const url = buildSearchUrl(limit, query);
|
||||
|
||||
try {
|
||||
const response = await fetch(url, {
|
||||
headers: { Accept: "application/json" },
|
||||
|
||||
Reference in New Issue
Block a user