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:
5
.changeset/fix-skills-catalog-400.md
Normal file
5
.changeset/fix-skills-catalog-400.md
Normal file
@@ -0,0 +1,5 @@
|
|||||||
|
---
|
||||||
|
"@runfusion/fusion": patch
|
||||||
|
---
|
||||||
|
|
||||||
|
Fix Skills Catalog initial-load failures by preventing unauthenticated public search requests for empty or too-short queries. The dashboard now returns a successful empty catalog result for short-query unauthenticated/fallback states instead of surfacing upstream 400 errors.
|
||||||
@@ -314,9 +314,14 @@ Fetch the skills.sh catalog with optional authentication.
|
|||||||
|
|
||||||
**Authentication Flow:**
|
**Authentication Flow:**
|
||||||
1. If `SKILLS_SH_TOKEN` env var is present, use authenticated request
|
1. If `SKILLS_SH_TOKEN` env var is present, use authenticated request
|
||||||
2. If authenticated request returns `401/403`, retry without authentication (fallback mode)
|
2. If authenticated request returns `400/401/403`, retry without authentication (fallback mode)
|
||||||
3. If no token, use unauthenticated request directly
|
3. If no token, use unauthenticated request directly
|
||||||
|
|
||||||
|
**Unauthenticated Short-Query Behavior:**
|
||||||
|
- Public `skills.sh /api/search` requests are only sent when `q` has at least 2 characters
|
||||||
|
- For omitted, empty, or 1-character queries, the API returns `200` with `{ entries: [] }`
|
||||||
|
- This applies both to direct unauthenticated mode and authenticated-to-unauthenticated fallback mode, preventing upstream `400 Bad Request` responses during initial load
|
||||||
|
|
||||||
**Auth Mode Values:**
|
**Auth Mode Values:**
|
||||||
- `authenticated` — Request made with token
|
- `authenticated` — Request made with token
|
||||||
- `unauthenticated` — Request made without token (no token available)
|
- `unauthenticated` — Request made without token (no token available)
|
||||||
|
|||||||
@@ -63,15 +63,34 @@ export function SkillsView({ projectId, addToast, onClose }: SkillsViewProps) {
|
|||||||
|
|
||||||
// Fetch catalog
|
// Fetch catalog
|
||||||
const loadCatalog = useCallback(async (query: string) => {
|
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);
|
setIsLoadingCatalog(true);
|
||||||
setCatalogError(null);
|
setCatalogError(null);
|
||||||
try {
|
try {
|
||||||
const result = await fetchSkillsCatalog(query, 20, projectId);
|
const result = await fetchSkillsCatalog(query, 20, projectId);
|
||||||
setCatalogEntries(result.entries);
|
setCatalogEntries(result.entries);
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
// Check for upstream error with code (502 etc.)
|
if (isCatalogUnavailableError(err)) {
|
||||||
const error = err as { error?: string; code?: string };
|
|
||||||
if (error.error && error.code) {
|
|
||||||
setCatalogError("Catalog is temporarily unavailable. Please try again later.");
|
setCatalogError("Catalog is temporarily unavailable. Please try again later.");
|
||||||
} else {
|
} else {
|
||||||
const message = err instanceof Error ? err.message : "Failed to load catalog";
|
const message = err instanceof Error ? err.message : "Failed to load catalog";
|
||||||
|
|||||||
@@ -336,11 +336,13 @@ describe("SkillsView", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
describe("error handling", () => {
|
describe("error handling", () => {
|
||||||
it("shows error message for catalog fetch with upstream error", async () => {
|
it("shows friendly error message for catalog fetch with upstream ApiRequestError", async () => {
|
||||||
mockFetchSkillsCatalog.mockRejectedValue({
|
mockFetchSkillsCatalog.mockRejectedValue(
|
||||||
error: "Service unavailable",
|
Object.assign(new Error("Upstream returned 400: Bad Request"), {
|
||||||
code: "upstream_http_error",
|
status: 502,
|
||||||
});
|
details: { code: "upstream_http_error" },
|
||||||
|
})
|
||||||
|
);
|
||||||
|
|
||||||
render(<SkillsView addToast={mockAddToast} onClose={onClose} />);
|
render(<SkillsView addToast={mockAddToast} onClose={onClose} />);
|
||||||
|
|
||||||
@@ -363,10 +365,12 @@ describe("SkillsView", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it("shows Try Again button for catalog error", async () => {
|
it("shows Try Again button for catalog error", async () => {
|
||||||
mockFetchSkillsCatalog.mockRejectedValue({
|
mockFetchSkillsCatalog.mockRejectedValue(
|
||||||
error: "Service unavailable",
|
Object.assign(new Error("Upstream returned 400: Bad Request"), {
|
||||||
code: "upstream_http_error",
|
status: 502,
|
||||||
});
|
details: { code: "upstream_http_error" },
|
||||||
|
})
|
||||||
|
);
|
||||||
|
|
||||||
render(<SkillsView addToast={mockAddToast} onClose={onClose} />);
|
render(<SkillsView addToast={mockAddToast} onClose={onClose} />);
|
||||||
|
|
||||||
@@ -569,10 +573,12 @@ describe("SkillsView", () => {
|
|||||||
|
|
||||||
describe("error-state retry", () => {
|
describe("error-state retry", () => {
|
||||||
it("retry button is displayed when catalog fetch fails", async () => {
|
it("retry button is displayed when catalog fetch fails", async () => {
|
||||||
mockFetchSkillsCatalog.mockRejectedValue({
|
mockFetchSkillsCatalog.mockRejectedValue(
|
||||||
error: "Service unavailable",
|
Object.assign(new Error("Upstream returned 400: Bad Request"), {
|
||||||
code: "upstream_http_error",
|
status: 502,
|
||||||
});
|
details: { code: "upstream_http_error" },
|
||||||
|
})
|
||||||
|
);
|
||||||
|
|
||||||
render(<SkillsView addToast={mockAddToast} onClose={onClose} />);
|
render(<SkillsView addToast={mockAddToast} onClose={onClose} />);
|
||||||
|
|
||||||
|
|||||||
@@ -531,7 +531,7 @@ describe("Skills routes", () => {
|
|||||||
expect(mockAdapter.fetchCatalog).toHaveBeenCalledWith({ limit: 20, query: "react" });
|
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({
|
const mockAdapter = createMockSkillsAdapter({
|
||||||
fetchCatalog: vi.fn().mockResolvedValue({
|
fetchCatalog: vi.fn().mockResolvedValue({
|
||||||
entries: [
|
entries: [
|
||||||
|
|||||||
@@ -215,6 +215,106 @@ describe("createSkillsAdapter - fetchCatalog fallback behavior", () => {
|
|||||||
expect(result.auth.fallbackUsed).toBe(false);
|
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", () => {
|
describe("createSkillsAdapter - readSkillContent", () => {
|
||||||
|
|||||||
@@ -418,11 +418,20 @@ export function createSkillsAdapter(options: {
|
|||||||
// Get skills.sh token if available
|
// Get skills.sh token if available
|
||||||
const token = process.env.SKILLS_SH_TOKEN;
|
const token = process.env.SKILLS_SH_TOKEN;
|
||||||
const catalogUrl = buildCatalogUrl(boundedLimit, query);
|
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) {
|
if (!token) {
|
||||||
return fetchPublicCatalog(searchUrl, {
|
if (!publicSearchQuery) {
|
||||||
|
return buildEmptyCatalogResult({
|
||||||
|
mode: "unauthenticated",
|
||||||
|
tokenPresent: false,
|
||||||
|
fallbackUsed: false,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
return fetchPublicCatalog(boundedLimit, publicSearchQuery, {
|
||||||
mode: "unauthenticated",
|
mode: "unauthenticated",
|
||||||
tokenPresent: false,
|
tokenPresent: false,
|
||||||
fallbackUsed: false,
|
fallbackUsed: false,
|
||||||
@@ -453,7 +462,15 @@ export function createSkillsAdapter(options: {
|
|||||||
|
|
||||||
// 400/401/403 from authenticated request - fall back to public search endpoint
|
// 400/401/403 from authenticated request - fall back to public search endpoint
|
||||||
if (authResponse.status === 400 || authResponse.status === 401 || authResponse.status === 403) {
|
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",
|
mode: "fallback-unauthenticated",
|
||||||
tokenPresent: true,
|
tokenPresent: true,
|
||||||
fallbackUsed: true,
|
fallbackUsed: true,
|
||||||
@@ -561,14 +578,11 @@ function buildCatalogUrl(limit: number, query?: string): string {
|
|||||||
/**
|
/**
|
||||||
* Build the public search URL.
|
* Build the public search URL.
|
||||||
*
|
*
|
||||||
* The search API requires a non-empty query. For initial catalog browsing,
|
* The search API requires a non-empty query that meets minimum length.
|
||||||
* use "*" as a wildcard query.
|
|
||||||
*/
|
*/
|
||||||
function buildSearchUrl(limit: number, query?: string): string {
|
function buildSearchUrl(limit: number, query: string): string {
|
||||||
const params = new URLSearchParams();
|
const params = new URLSearchParams();
|
||||||
const normalizedQuery = query?.trim();
|
params.set("q", query);
|
||||||
|
|
||||||
params.set("q", normalizedQuery && normalizedQuery.length > 0 ? normalizedQuery : "*");
|
|
||||||
params.set("limit", String(limit));
|
params.set("limit", String(limit));
|
||||||
|
|
||||||
return `https://skills.sh/api/search?${params.toString()}`;
|
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.
|
* 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(
|
async function fetchPublicCatalog(
|
||||||
url: string,
|
limit: number,
|
||||||
|
query: string,
|
||||||
auth: CatalogFetchResult["auth"],
|
auth: CatalogFetchResult["auth"],
|
||||||
): Promise<CatalogFetchResult | UpstreamError> {
|
): Promise<CatalogFetchResult | UpstreamError> {
|
||||||
|
const url = buildSearchUrl(limit, query);
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const response = await fetch(url, {
|
const response = await fetch(url, {
|
||||||
headers: { Accept: "application/json" },
|
headers: { Accept: "application/json" },
|
||||||
|
|||||||
Reference in New Issue
Block a user