fix: overhaul Google Books API (429/403/no-results) + add Open Library provider#158
Open
oRadiatorExtrem wants to merge 1 commit into
Open
fix: overhaul Google Books API (429/403/no-results) + add Open Library provider#158oRadiatorExtrem wants to merge 1 commit into
oRadiatorExtrem wants to merge 1 commit into
Conversation
- Fix 429/403 errors: use requestUrl throw:false, intercept status codes with clear user-facing messages - Fix langRestrict bug: normalize locale (en-US->en), skip restriction on default locale -- root cause of issue anpigon#153 (no results) - Fix ISBN search: strip dashes, use isbn: prefix, skip langRestrict - Add coverMediumUrl/coverLargeUrl from Google Books zoom parameter - Add Open Library as new provider (no API key, free, global) with ISBN search and S/M/L cover images - Improve API key test button: does real search instead of string check - Add warning about Google Books rate limiting without API key - Format all files with prettier (pre-existing lint debt) - Bump version to 0.8.0 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
WalkthroughThis PR adds Open Library as a third book search provider to the plugin, extending the existing Google Books and Naver support. It includes HTTP error handling improvements, ISBN detection and locale refactoring in Google Books, the complete Open Library API implementation, UI configuration wiring, and build configuration updates for version 0.8.0. ChangesOpen Library Search Provider
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Review Summary by QodoFix Google Books API errors and add Open Library provider
WalkthroughsDescription• Fix 429/403 rate-limit errors with explicit HTTP status handling • Fix langRestrict bug: normalize locales, skip on default locale • Fix ISBN search: use isbn: prefix, strip dashes, skip langRestrict • Add Open Library provider: free, no API key, global coverage • Add medium/large cover URLs via Google Books zoom parameter • Improve API key test: performs real search instead of string check Diagramflowchart LR
A["HTTP Requests"] -->|"throw: false + status checks"| B["Error Handling"]
B -->|"429/403/4xx"| C["User-Friendly Messages"]
D["Query Processing"] -->|"normalize locale"| E["Google Books API"]
D -->|"ISBN detection"| F["ISBN Search"]
E -->|"zoom parameter"| G["Cover URLs S/M/L"]
H["Open Library API"] -->|"no key needed"| I["Alternative Provider"]
J["Settings"] -->|"real search test"| K["API Key Validation"]
File Changes1. src/apis/base_api.ts
|
Code Review by Qodo
Context used 1. apiGet() missing 429 cooldown
|
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds Open Library as an additional book search provider and improves Google Books API handling, including better key testing and error handling.
Changes:
- Introduce
openLibraryas a newServiceProviderand add anOpenLibraryApiimplementation. - Improve Google Books search parameter building (ISBN handling, locale normalization) and add a real “Test API Key” flow in settings.
- Harden
apiGeterror handling and add pnpm build allowlisting foresbuild.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/settings/settings.ts | Adds Open Library option and implements a real Google Books API key test in settings UI. |
| src/constants.ts | Introduces ServiceProvider.openLibrary. |
| src/apis/open_library_api.ts | New Open Library provider implementation backed by /search.json. |
| src/apis/models/open_library_response.ts | Adds type definitions for Open Library search results. |
| src/apis/google_books_api.ts | Improves ISBN detection, locale normalization, and cover URL extraction helpers. |
| src/apis/base_api.ts | Adds Open Library to factory and centralizes HTTP status handling for API calls. |
| pnpm-workspace.yaml | Adds pnpm build allowlist configuration. |
| package.json | Bumps package version to 0.8.0. |
| manifest.json | Bumps plugin manifest version to 0.8.0. |
| .npmrc | Adds onlyBuiltDependencies allowlist entry for esbuild. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+59
to
+69
| if (res.status === 429) { | ||
| throw new Error( | ||
| 'Rate limit reached. Add a Google Books API key in Settings → Google API Settings, or switch to Open Library (no key needed).', | ||
| ); | ||
| } | ||
| if (res.status === 403) { | ||
| throw new Error('Access denied (403). Check your Google Books API key in Settings → Google API Settings.'); | ||
| } | ||
| if (res.status >= 400) { | ||
| throw new Error(`Request failed, status ${res.status}`); | ||
| } |
Comment on lines
+1
to
+2
| allowBuilds: | ||
| esbuild: true |
Comment on lines
1
to
5
| export enum ServiceProvider { | ||
| google = 'google', | ||
| naver = 'naver', | ||
| openLibrary = 'openLibrary', | ||
| } |
| import { Book } from '@models/book.model'; | ||
| import { OpenLibraryDoc, OpenLibrarySearchResponse } from './models/open_library_response'; | ||
|
|
||
| const ISBN_RE = /^(97[89])?\d{9}[\dX]$/i; |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/apis/base_api.ts`:
- Around line 59-66: The 429/403 error messages in apiGet are hard-coded for
Google and therefore show incorrect remediation for other providers; update the
apiGet implementation to generate provider-aware messages (use the function's
provider/source parameter or derive the provider from the request URL) and
branch on possible values (e.g., "google", "naver", "openlibrary") so 429 errors
advise adding/configuring a key only for Google/Naver and 403 errors reference
the correct provider-specific settings (Google API Settings vs Naver API
settings) or mention that Open Library requires no key; replace the current
throw new Error(...) calls for res.status === 429 and res.status === 403 with
messages selected based on that provider detection.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 369b4366-bcc0-4d74-8079-3b1fa29bef35
📒 Files selected for processing (10)
.npmrcmanifest.jsonpackage.jsonpnpm-workspace.yamlsrc/apis/base_api.tssrc/apis/google_books_api.tssrc/apis/models/open_library_response.tssrc/apis/open_library_api.tssrc/constants.tssrc/settings/settings.ts
Comment on lines
+59
to
+66
| if (res.status === 429) { | ||
| throw new Error( | ||
| 'Rate limit reached. Add a Google Books API key in Settings → Google API Settings, or switch to Open Library (no key needed).', | ||
| ); | ||
| } | ||
| if (res.status === 403) { | ||
| throw new Error('Access denied (403). Check your Google Books API key in Settings → Google API Settings.'); | ||
| } |
There was a problem hiding this comment.
Make shared 429/403 errors provider-aware.
At Line 59 and Line 65, apiGet always tells users to fix Google API settings. This function is shared by Google, Naver, and Open Library, so non-Google failures will show incorrect remediation.
Suggested fix
- if (res.status === 429) {
- throw new Error(
- 'Rate limit reached. Add a Google Books API key in Settings → Google API Settings, or switch to Open Library (no key needed).',
- );
- }
- if (res.status === 403) {
- throw new Error('Access denied (403). Check your Google Books API key in Settings → Google API Settings.');
- }
+ if (res.status === 429) {
+ if (apiURL.hostname.includes('googleapis.com')) {
+ throw new Error(
+ 'Rate limit reached. Add a Google Books API key in Settings → Google API Settings, or switch to Open Library (no key needed).',
+ );
+ }
+ throw new Error(`Rate limit reached (429) from ${apiURL.hostname}.`);
+ }
+ if (res.status === 403) {
+ if (apiURL.hostname.includes('googleapis.com')) {
+ throw new Error('Access denied (403). Check your Google Books API key in Settings → Google API Settings.');
+ }
+ throw new Error(`Access denied (403) from ${apiURL.hostname}.`);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (res.status === 429) { | |
| throw new Error( | |
| 'Rate limit reached. Add a Google Books API key in Settings → Google API Settings, or switch to Open Library (no key needed).', | |
| ); | |
| } | |
| if (res.status === 403) { | |
| throw new Error('Access denied (403). Check your Google Books API key in Settings → Google API Settings.'); | |
| } | |
| if (res.status === 429) { | |
| if (apiURL.hostname.includes('googleapis.com')) { | |
| throw new Error( | |
| 'Rate limit reached. Add a Google Books API key in Settings → Google API Settings, or switch to Open Library (no key needed).', | |
| ); | |
| } | |
| throw new Error(`Rate limit reached (429) from ${apiURL.hostname}.`); | |
| } | |
| if (res.status === 403) { | |
| if (apiURL.hostname.includes('googleapis.com')) { | |
| throw new Error('Access denied (403). Check your Google Books API key in Settings → Google API Settings.'); | |
| } | |
| throw new Error(`Access denied (403) from ${apiURL.hostname}.`); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/apis/base_api.ts` around lines 59 - 66, The 429/403 error messages in
apiGet are hard-coded for Google and therefore show incorrect remediation for
other providers; update the apiGet implementation to generate provider-aware
messages (use the function's provider/source parameter or derive the provider
from the request URL) and branch on possible values (e.g., "google", "naver",
"openlibrary") so 429 errors advise adding/configuring a key only for
Google/Naver and 403 errors reference the correct provider-specific settings
(Google API Settings vs Naver API settings) or mention that Open Library
requires no key; replace the current throw new Error(...) calls for res.status
=== 429 and res.status === 403 with messages selected based on that provider
detection.
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
Comment on lines
+59
to
+63
| if (res.status === 429) { | ||
| throw new Error( | ||
| 'Rate limit reached. Add a Google Books API key in Settings → Google API Settings, or switch to Open Library (no key needed).', | ||
| ); | ||
| } |
There was a problem hiding this comment.
1. apiget() missing 429 cooldown 📎 Requirement gap ☼ Reliability
The new 429 handling throws immediately without any backoff/cooldown, so repeated searches will continue failing under persistent rate limiting. This does not provide a recovery path across sessions/devices as required.
Agent Prompt
## Issue description
`apiGet()` detects HTTP 429 but immediately throws an error and does not implement any cooldown/backoff. This means users who hit rate limits can keep failing on every subsequent search with no mitigation.
## Issue Context
Compliance requires reducing request frequency and/or using backoff/cooldown so searches can resume after waiting, with consistent behavior across sessions/devices.
## Fix Focus Areas
- src/apis/base_api.ts[40-69]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Test plan
Generated with Claude Code
Summary by CodeRabbit