Skip to content

Conversation

junseok44
Copy link
Contributor

related to #955

Problem

The OpenProcessing API was experiencing 429 rate limiting errors during build due to excessive API calls. Investigation revealed that sketch memoization was failing because of a type mismatch between the API response (visualID: number) and the comparison logic (string).

Changes

  • Changed visualID type from string to number
  • Removed unnecessary toString() calls in HomepageLayout
  • Explicitly convert to string when creating URL parameters (String(sketch.visualID)) to prevent TypeScript from inferring Astro.params.slug as number
  • Added centralized type conversion in SketchLayout (const sketchIdNumber = Number(sketchId))
  • Updated all OpenProcessing helper functions to accept number parameters (previously string)

I considered two approaches:

  1. Keep each function in OpenProcessing.ts accepting string and convert internally to number
  2. Convert sketchId to number once in the SketchLayout and have functions accept number as parameters

I went with option 2 because I thought it's more consistent with the visualID type and avoids multiple conversions per function call. Let me know if you think a different approach would be better.

Future Work

Currently, I added basic validation for invalid sketch IDs in SketchLayout.astro:

const sketchIdNumber = Number(sketchId);

if (isNaN(sketchIdNumber)) {
  console.error(`Invalid sketch ID: ${sketchId}`);
}

However, this only logs the error and continues execution, potentially passing NaN to OpenProcessing functions.
I think A follow-up PR should address proper error handling such as:

  • Conditional rendering to show error page
  • Pre-filtering invalid IDs in getStaticPaths
  • Graceful fallback mechanisms....etc

Thanks for reviewing!

- Change visualID type from string to number to match API response
- Update all OpenProcessing functions to accept number parameters
- Add explicit String() conversion in getStaticPaths for URL compatibility
- Remove unnecessary type conversions in sketch lookups
- Enable proper memoization to reduce API calls and prevent 429 errors
@ksen0 ksen0 merged commit df637e9 into processing:main Sep 8, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants