Skip to content

fix: improve error logging to serialize Response objects #1115

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.

Sign up for GitHub

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

Open
igoramf wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: improve error logging to serialize Response objects #1115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions clients/withManifest.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,18 +158,22 @@ const fetchWithProps = async (
return response.json();
}

console.error(init?.body, response);
const error = await response.text();
const errorBody = await response.text();
const correlationId = response.headers.get("x-correlation-id");
console.error(
`Invoke error ${response.status} ${response.statusText} - ${url}`,
errorBody,
);
if (response.headers.get("content-type") === "application/json") {
const errorObj = JSON.parse(error);
const errorObj = JSON.parse(errorBody);
throw new InvokeError(`${response.status}: ${response.statusText}`, {
cause: errorObj,
correlationId: response.headers.get("x-correlation-id"),
correlationId,
});
Comment on lines 167 to 172
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, locate the file and examine the specific lines mentioned
git ls-files | grep -E "withManifest\.(ts|js)$"

Repository: deco-cx/deco

Length of output: 79

🏁 Script executed:

# Read the file around the specified lines
wc -l clients/withManifest.ts

Repository: deco-cx/deco

Length of output: 83

🏁 Script executed:

# Get context around lines 167-172
sed -n '155,180p' clients/withManifest.ts

Repository: deco-cx/deco

Length of output: 815

🏁 Script executed:

# Search for other JSON.parse calls in the codebase to see error handling patterns
rg 'JSON\.parse' -B 2 -A 2 --type ts --type js

Repository: deco-cx/deco

Length of output: 13161

🏁 Script executed:

# Check the InvokeError definition and how correlationId is used
rg 'class InvokeError|interface InvokeError' -A 10 --type ts

Repository: deco-cx/deco

Length of output: 627

🏁 Script executed:

# Check if there's any error handling around JSON parsing elsewhere
rg 'try.*JSON\.parse|JSON\.parse.*catch' --type ts --type js

Repository: deco-cx/deco

Length of output: 774

Guard JSON error parsing and accept charset-suffixed JSON.

The current branch only matches an exact application/json header, which misses common variants like application/json; charset=utf-8. Additionally, JSON.parse(errorBody) has no guard, so malformed or empty JSON bodies throw a raw SyntaxError, causing callers to lose the normalized InvokeError path and its correlationId for observability.

Proposed fix 
+  const contentType = response.headers.get("content-type") ?? "";
-  if (response.headers.get("content-type") === "application/json") {
-    const errorObj = JSON.parse(errorBody);
+  if (contentType.startsWith("application/json")) {
+    let errorObj: unknown = errorBody;
+    try {
+      errorObj = errorBody ? JSON.parse(errorBody) : null;
+    } catch {
+      // Fall back to the raw body and preserve correlationId on InvokeError.
+    }
     throw new InvokeError(`${response.status}: ${response.statusText}`, {
       cause: errorObj,
       correlationId,
     });
   }
📝 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 (response.headers.get("content-type") === "application/json") {
const errorObj = JSON.parse(error);
const errorObj = JSON.parse(errorBody);
throw new InvokeError(`${response.status}: ${response.statusText}`, {
cause: errorObj,
correlationId: response.headers.get("x-correlation-id"),
correlationId,
});
const contentType = response.headers.get("content-type") ?? "";
if (contentType.startsWith("application/json")) {
let errorObj: unknown = errorBody;
try {
errorObj = errorBody ? JSON.parse(errorBody) : null;
} catch {
// Fall back to the raw body and preserve correlationId on InvokeError.
}
throw new InvokeError(`${response.status}: ${response.statusText}`, {
cause: errorObj,
correlationId,
});
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@clients/withManifest.ts` around lines 167 - 172, The content-type check and
JSON parsing need to be made robust: change the header check to detect JSON
media types with parameters (e.g., use
response.headers.get("content-type")?.includes("application/json") or
startsWith) and wrap JSON.parse(errorBody) in a try/catch; if parse succeeds use
the parsed object as the InvokeError cause, otherwise fall back to using the raw
errorBody (or undefined) as the cause so you still throw the normalized
InvokeError with the correlationId. Ensure you update the branch around
response, errorBody, InvokeError, and correlationId to always throw an
InvokeError (with correlationId) even when JSON parsing fails.

}
throw new InvokeError(`${response.status}: ${response.statusText}`, {
cause: error,
correlationId: response.headers.get("x-correlation-id"),
cause: errorBody,
correlationId,
});
};

Expand Down
6 changes: 5 additions & 1 deletion utils/fetchAPI.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ export const fetchAPI = async <T>(
return response.json();
}

console.error(input, response);
const errorBody = await response.text();
console.error(
`fetchAPI error ${response.status} ${response.statusText} - ${input}`,
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Mar 10, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Log the request URL explicitly for Request inputs; interpolating input directly still serializes as [object Request].

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At utils/fetchAPI.ts, line 19:

<comment>Log the request URL explicitly for `Request` inputs; interpolating `input` directly still serializes as `[object Request]`.</comment>

<file context>
@@ -14,6 +14,10 @@ export const fetchAPI = async <T>(
-  console.error(input, response);
+  const errorBody = await response.text();
+  console.error(
+    `fetchAPI error ${response.status} ${response.statusText} - ${input}`,
+    errorBody,
+  );
</file context>
Suggested change
`fetchAPI error ${response.status} ${response.statusText} - ${input}`,
`fetchAPI error ${response.status} ${response.statusText} - ${typeof input === "string" ? input : input.url}`,
Fix with Cubic

errorBody,
);
throw new Error(`fetch ${input} responded with status ${response.status}`);
};
Loading