Improve OpenProcessing API error handling and add caching tests

src/api/OpenProcessing.ts

@@ -33,6 +33,9 @@ export type OpenProcessingCurationResponse = Array<{
   fullname: string;
 }>;
 
+// Selected Sketches from the 2025 curation
+export const priorityIds = ['2690038', '2484739', '2688829', '2689119', '2690571', '2690405','2684408' , '2693274', '2693345', '2691712']
+
 /**
  * Get basic info for the sketches contained in a Curation
  * from the OpenProcessing API
@@ -47,21 +50,20 @@ export const getCurationSketches = memoize(async (
   const response1 = await fetch(
     `${openProcessingEndpoint}curation/${curationId}/sketches?${limitParam}`,
   );
-  if(!response1.ok){ //log error instead of throwing error to not cache result in memoize
-    console.error('getCurationSketches', response1.status, response1.statusText)
+  if(!response1.ok){ 
+    throw new Error(`getCurationSketches: ${response1.status} ${response1.statusText}`)
   }
   const payload1 = await response1.json();
 
   const response2 = await fetch(
     `${openProcessingEndpoint}curation/${newCurationId}/sketches?${limitParam}`,
   );
-  if(!response2.ok){ //log error instead of throwing error to not cache result in memoize
-    console.error('getCurationSketches', response2.status, response2.statusText)
+  if(!response2.ok){ 
+    throw new Error(`getCurationSketches: ${response2.status} ${response2.statusText}`)
   }
   const payload2 = await response2.json();
 
-  // Selected Sketches from the 2025 curation
-  const priorityIds = ['2690038', '2484739', '2688829', '2689119', '2690571', '2690405','2684408' , '2693274', '2693345', '2691712']
+
 
   const prioritySketches = payload2.filter(
     (sketch: OpenProcessingCurationResponse[number]) => priorityIds.includes(String(sketch.visualID)))
@@ -122,8 +124,7 @@ export const getSketch = memoize(
     // check for sketch data in Open Processing API
   const response = await fetch(`${openProcessingEndpoint}sketch/${id}`);
     if (!response.ok) {
-      //log error instead of throwing error to not cache result in memoize
-      console.error("getSketch", id, response.status, response.statusText);
+      throw new Error(`getSketch: ${id} ${response.status} ${response.statusText}`)
   }
   const payload = await response.json();
   return payload as OpenProcessingSketchResponse;
@@ -141,8 +142,8 @@ export const getSketchSize = memoize(async (id: number) => {
   }
 
   const response = await fetch(`${openProcessingEndpoint}sketch/${id}/code`);
-  if(!response.ok){ //log error instead of throwing error to not cache result in memoize
-    console.error('getSketchSize', id, response.status, response.statusText)
+  if(!response.ok){ 
+    throw new Error(`getSketchSize: ${id} ${response.status} ${response.statusText}`)
   }
   const payload = await response.json();
 

src/layouts/SketchLayout.astro

@@ -24,10 +24,6 @@ const { sketchId, authorName } = Astro.props;
 
 const sketchIdNumber = Number(sketchId);
 
-if (isNaN(sketchIdNumber)) {
-  console.error(`Invalid sketch ID: ${sketchId}`);
-}
-
 const { title, createdOn, instructions } = await getSketch(sketchIdNumber);
 
 const currentLocale = getCurrentLocale(Astro.url.pathname);

src/pages/[locale]/sketches/[...slug].astro

@@ -7,7 +7,7 @@ export async function getStaticPaths() {
   const sketches = await getCurationSketches();
   const entries = await Promise.all(
     nonDefaultSupportedLocales.map(async (locale) => {
-      return sketches.map((sketch) => ({
+      return sketches.filter(sketch => sketch.visualID && typeof sketch.visualID === 'number').map((sketch) => ({
         // Even though slug gets converted to string at runtime,
         // TypeScript infers it as number from sketch.visualID, so we explicitly convert to string.
         params: { locale, slug: String(sketch.visualID) },

src/pages/sketches/[...slug].astro

@@ -4,7 +4,7 @@ import { getCurationSketches } from "@src/api/OpenProcessing";
 
 export async function getStaticPaths() {
   const sketches = await getCurationSketches();
-  return sketches.map((sketch) => ({
+  return sketches.filter(sketch => sketch.visualID && typeof sketch.visualID === 'number').map((sketch) => ({
     // Even though slug gets converted to string at runtime,
     // TypeScript infers it as number from sketch.visualID, so we explicitly convert to string.
     params: { slug: String(sketch.visualID) },

test/api/OpenProcessing.test.ts

@@ -0,0 +1,231 @@
+// src/api/OpenProcessing.test.ts
+
+import { getCurationSketches, getSketch, getSketchSize, priorityIds, type OpenProcessingCurationResponse } from '@/src/api/OpenProcessing';
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+
+const mockFetch = vi.fn();
+
+vi.stubGlobal('fetch', mockFetch);
+
+// Test data: first item is mock data, second uses actual priority ID from current curation
+const getCurationSketchesData : OpenProcessingCurationResponse = [{
+    visualID: 101,
+    title: 'Sketch One',
+    description: 'Description One',
+    instructions: 'Instructions One',
+    mode: 'p5js',
+    createdOn: '2025-01-01',
+    userID: 'User One',
+    submittedOn: '2025-01-01',
+    fullname: 'Fullname One'
+}, 
+{
+    visualID: Number(priorityIds[0]), // Real ID from current curation priority list
+    title: 'Sketch Two',
+    description: 'Description Two',
+    instructions: 'Instructions Two',
+    mode: 'p5js',
+    createdOn: '2025-01-01',
+    userID: 'User Two',
+    submittedOn: '2025-01-01',
+    fullname: 'Fullname Two'
+}]
+
+describe('OpenProcessing API Caching', () => {
+
+  beforeEach(() => {
+    vi.clearAllMocks();
+
+    getCurationSketches.cache.clear?.();
+    getSketch.cache.clear?.();
+    getSketchSize.cache.clear?.();
+  });
+
+  // Case 1: Verify caching for getCurationSketches
+  it('should only call the API once even if getCurationSketches is called multiple times', async () => {
+
+    mockFetch.mockResolvedValue({
+      ok: true,
+      json: () => Promise.resolve(getCurationSketchesData),
+    });
+
+    await getCurationSketches();
+    await getCurationSketches();
+
+    // Check if fetch was called exactly 2 times (for the two curation IDs).
+    // If this number becomes 4, it means the caching is broken.
+    expect(mockFetch).toHaveBeenCalledTimes(2);
+  });
+
+  // Case 2: Verify getSketch uses cached data from getCurationSketches
+  it('should use cached data from getCurationSketches for getSketch calls', async () => {
+
+    mockFetch.mockResolvedValueOnce({ // for curationId
+      ok: true,
+      json: () => Promise.resolve([getCurationSketchesData[0]]),
+    }).mockResolvedValueOnce({ // for newCurationId
+      ok: true,
+      json: () => Promise.resolve([getCurationSketchesData[1]]),
+    });
+
+    // Call the main function to populate the cache.
+    await getCurationSketches();
+    // At this point, fetch has been called twice.
+    expect(mockFetch).toHaveBeenCalledTimes(2);
+
+    // Now, call getSketch with an ID that should be in the cache.
+    const sketch = await getSketch(getCurationSketchesData[0].visualID);
+    expect(sketch.title).toBe('Sketch One');
+    const sketch2 = await getSketch(getCurationSketchesData[1].visualID);
+    expect(sketch2.title).toBe('Sketch Two');
+
+    // Verify that no additional fetch calls were made.
+    // The call count should still be 2 because the data came from the cache.
+    expect(mockFetch).toHaveBeenCalledTimes(2);
+  });
+
+  // Case 3: Verify getSketch fetches individual sketch when not in cache
+  it('should fetch individual sketch when not in cache', async () => {
+
+    // for curationId
+    mockFetch.mockResolvedValueOnce({
+      ok: true,
+      json: () => Promise.resolve([])
+    }).mockResolvedValueOnce({ // for newCurationId
+      ok: true, 
+      json: () => Promise.resolve([])
+    });
+
+    await getCurationSketches(); // Create empty cache
+
+    // Individual sketch API call
+    mockFetch.mockResolvedValueOnce({
+      ok: true,
+      json: () => Promise.resolve({ visualID: 999, title: 'Individual Sketch' })
+    });
+
+    const sketch = await getSketch(999);
+    expect(sketch.title).toBe('Individual Sketch');
+    expect(mockFetch).toHaveBeenCalledTimes(3); // 2 for empty curations in getCurationSketches + 1 for individual call in getSketch
+  });
+
+  // Case 4: Overall regression test for total sketch page generation
+  it('should not exceed the expected number of API calls during a build simulation', async () => {
+
+    // Mock the responses for getCurationSketches.
+    mockFetch.mockResolvedValueOnce({ // for curationId
+      ok: true,
+      json: () => Promise.resolve([getCurationSketchesData[0]]),
+    }).mockResolvedValueOnce({ // for newCurationId
+      ok: true,
+      json: () => Promise.resolve([getCurationSketchesData[1]]),
+    });
+
+    // 2. Mock the response for getSketchSize calls.
+    mockFetch.mockResolvedValue({ // for all subsequent calls
+      ok: true,
+      json: () => Promise.resolve([{ code: 'createCanvas(400, 400);' }]),
+    });
+
+    // --- sketch page build simulation ---
+    // This simulates what happens during `getStaticPaths`.
+    const sketches = await getCurationSketches(); // Makes 2 API calls.
+
+    // This simulates what happens as each page is generated.
+    for (const sketch of sketches) {
+      // Inside the page component, getSketch and getSketchSize would be called.
+      await getSketch(sketch.visualID); // Uses cache (0 new calls).
+      await getSketchSize(sketch.visualID); // Makes 1 new API call.
+    }
+    // --- simulation end ---
+
+    // Calculate the total expected calls.
+    // 2 for getCurationSketches + 1 for each sketch's getSketchSize call.
+    const expectedCalls = 2 + sketches.length;
+    expect(mockFetch).toHaveBeenCalledTimes(expectedCalls);
+
+  });
+});
+
+describe('Error Handling', () => {
+
+    beforeEach(() => {
+        vi.clearAllMocks();
+
+        getCurationSketches.cache.clear?.();
+        getSketch.cache.clear?.();
+        getSketchSize.cache.clear?.();
+      });
+
+    it('should throw an error when getCurationSketches API call fails', async () => {
+      mockFetch.mockResolvedValue({
+        ok: false,
+        status: 500,
+        statusText: 'Internal Server Error',
+      });
+
+      await expect(getCurationSketches()).rejects.toThrow(
+        'getCurationSketches: 500 Internal Server Error'
+      );
+    });
+
+    it('should throw an error when rate limit is exceeded (429)', async () => {
+      mockFetch.mockResolvedValue({
+        ok: false,
+        status: 429,
+        statusText: 'Too Many Requests',
+      });
+
+      await expect(getCurationSketches()).rejects.toThrow(
+        'getCurationSketches: 429 Too Many Requests'
+      );
+    });
+
+    it('should throw an error when getSketch API call fails for individual sketch', async () => {
+      // Setup empty curations first
+      mockFetch.mockResolvedValueOnce({
+        ok: true,
+        json: () => Promise.resolve([])
+      }).mockResolvedValueOnce({
+        ok: true,
+        json: () => Promise.resolve([])
+      });
+
+      await getCurationSketches(); // Create empty cache
+
+      // Individual sketch API call fails with 429
+      mockFetch.mockResolvedValueOnce({
+        ok: false,
+        status: 429,
+        statusText: 'Too Many Requests'
+      });
+
+      await expect(getSketch(999)).rejects.toThrow(
+        'getSketch: 999 429 Too Many Requests'
+      );
+    });
+
+    it('should throw an error when getSketchSize API call fails', async () => {
+      // Setup sketch data first
+      mockFetch.mockResolvedValueOnce({
+        ok: true,
+        json: () => Promise.resolve([getCurationSketchesData[0]])
+      }).mockResolvedValueOnce({
+        ok: true,
+        json: () => Promise.resolve([])
+      });
+
+      await getCurationSketches();
+
+      // getSketchSize API call fails with rate limit
+      mockFetch.mockResolvedValueOnce({
+        ok: false,
+        status: 429,
+        statusText: 'Too Many Requests'
+      });
+
+      await expect(getSketchSize(getCurationSketchesData[0].visualID)).rejects.toThrow(
+        `getSketchSize: ${getCurationSketchesData[0].visualID} 429 Too Many Requests`
+      );
+    });
+  });