-
Notifications
You must be signed in to change notification settings - Fork 200
feat:API robustness enhancement#3670 #309
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
42d1e17
991771c
7589ed1
79d3b5d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -3,30 +3,78 @@ export const dynamic = 'force-dynamic'; | |||||||||
|
|
||||||||||
| const API_URL = process.env.WORDPRESS_API_URL || process.env.NEXT_PUBLIC_WORDPRESS_API_URL | ||||||||||
|
|
||||||||||
| async function fetchAPI(query = "", { variables }: Record<string, any> = {}) { | ||||||||||
| const headers = { "Content-Type": "application/json" }; | ||||||||||
| const DEFAULT_TIMEOUT_MS = 10000; | ||||||||||
| const envTimeoutMs = Number.parseInt( | ||||||||||
| process.env.WORDPRESS_API_TIMEOUT_MS || "", | ||||||||||
| 10 | ||||||||||
| ); | ||||||||||
| // can be overidden by setting the env , else the default will be used | ||||||||||
| const DEFAULT_REQUEST_TIMEOUT_MS = Number.isFinite(envTimeoutMs) | ||||||||||
| ? envTimeoutMs | ||||||||||
| : DEFAULT_TIMEOUT_MS; | ||||||||||
|
|
||||||||||
| async function fetchAPI( | ||||||||||
| query = "", | ||||||||||
| { variables, timeoutMs = DEFAULT_REQUEST_TIMEOUT_MS }: Record<string, any> = {} | ||||||||||
| ) { | ||||||||||
|
Comment on lines
+16
to
+19
|
||||||||||
| if (!API_URL) { | ||||||||||
| throw new Error("WORDPRESS_API_URL is not configured"); | ||||||||||
|
||||||||||
| throw new Error("WORDPRESS_API_URL is not configured"); | |
| throw new Error( | |
| "Neither WORDPRESS_API_URL nor NEXT_PUBLIC_WORDPRESS_API_URL is configured. Set one of them to your WordPress WPGraphQL endpoint." | |
| ); |
Copilot
AI
Apr 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
timeoutMs is accepted as any and can be overridden by callers; if it’s non-finite/<=0, setTimeout may behave unexpectedly (immediate abort / no timeout). Consider validating/coercing timeoutMs to a finite positive integer before using it, and tightening the options type to prevent accidental misuse.
Copilot
AI
Apr 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the server responds with a non-JSON Content-Type but a 2xx status, this function will currently return undefined (since json stays null) and callers will fail later with less context. Consider explicitly throwing when !isJson (or when json is null) for successful responses, including a short body snippet for diagnosis.
Copilot
AI
Apr 6, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This path logs full json.errors to stderr and then throws an error containing (a truncated) JSON string. That can lead to duplicate logging and may leak verbose upstream details into logs. Consider removing the console.error, or truncating/sanitizing what gets logged/thrown (and relying on the thrown error for diagnostics).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Typo/grammar in this comment: "overidden" -> "overridden" (also consider removing extra spaces around punctuation for readability).