feat: enhance error handling for chat

[SuZhou-Joe]

Description

The change adds error detection for ML client so it will return the error to users instead of trying to parse the response.body.

Issues Resolved

Screenshot

N/A

Before the fix

The server response with 500 error with

{
    "error": "Internal Server Error",
    "message": "Unexpected token 'F', \"Failed to \"... is not valid JSON",
    "statusCode": 500
}

After the fix

The server response with correct error code from server.

Testing the changes

Changelog

• feat: enhance error handling for chat

Check List

• All tests pass
  • yarn test:jest
  • yarn test:jest_integration
• New functionality includes testing.
• New functionality has been documented.
• Update CHANGELOG.md
• Commits are signed per the DCO using --signoff

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit a2dcd2d)

Here are some key observations to aid the review process:

🧪 No relevant tests

🔒 No security concerns identified

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Error Body Handling The MLClientError is constructed with mlResponse.body directly, but mlResponse.body could be an object, a string, or undefined. The Error constructor expects a string message, so passing an object may result in [object Object] as the error message. Consider serializing the body appropriately (e.g., JSON.stringify(mlResponse.body)) or extracting a meaningful message from it. if (mlResponse.status && mlResponse.status >= 400) { const mlClientError = new MLClientError(mlResponse.body); mlClientError.statusCode = mlResponse.status; mlClientError.statusText = mlResponse.statusText; throw mlClientError; } Status Check Logic The condition mlResponse.status && mlResponse.status >= 400 will not catch errors if mlResponse.status is 0 or falsy for some reason. Additionally, if mlResponse.status is undefined (e.g., for non-HTTP errors), the check silently passes. Consider using a more explicit check such as typeof mlResponse.status === 'number' && mlResponse.status >= 400 to ensure correctness. if (mlResponse.status && mlResponse.status >= 400) { const mlClientError = new MLClientError(mlResponse.body); mlClientError.statusCode = mlResponse.status; mlClientError.statusText = mlResponse.statusText; throw mlClientError; }

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to a2dcd2d
Explore these optional code suggestions:

Category	Suggestion	Impact
General	Add constructor with proper error initialization The MLClientError class doesn't call super() with a message or set the error name, which can make debugging harder. Add a constructor that accepts a message parameter, passes it to super(), and sets this.name to ensure proper error identification and stack traces. src/plugins/chat/server/routes/ml_routes/ml_client_error.ts [6-9] export class MLClientError extends Error { public statusCode?: number; public statusText?: string; + + constructor(message?: string) { + super(message); + this.name = 'MLClientError'; + } } Suggestion importance[1-10]: 6 __ Why: Adding a proper constructor with super(message) and this.name improves error identification and stack traces, which is a meaningful improvement for debugging. However, the current code still works functionally without it.	Low
Possible issue	Serialize error body to string properly mlResponse.body may be an object rather than a string, but Error messages should be strings. Passing an object directly to the Error constructor will result in [object Object] as the message. Serialize the body to a string before passing it to MLClientError. src/plugins/chat/server/routes/ml_routes/generic_ml_router.ts [122-127] if (mlResponse.status && mlResponse.status >= 400) { - const mlClientError = new MLClientError(mlResponse.body); + const bodyMessage = typeof mlResponse.body === 'string' ? mlResponse.body : JSON.stringify(mlResponse.body); + const mlClientError = new MLClientError(bodyMessage); mlClientError.statusCode = mlResponse.status; mlClientError.statusText = mlResponse.statusText; throw mlClientError; } Suggestion importance[1-10]: 5 __ Why: This is a valid concern since mlResponse.body could be an object, causing [object Object] as the error message. However, this only matters if suggestion 1 is also applied (adding a constructor that passes the message to super()), since the current MLClientError class has no constructor that uses the passed value.	Low

---

Previous suggestions

Suggestions up to commit a2dcd2d

Category	Suggestion	Impact
Possible issue	Handle non-string error body properly When mlResponse.body is an object (not a string), passing it directly to MLClientError will result in [object Object] as the error message. Convert the body to a string or extract a meaningful message field before passing it to the error constructor. src/plugins/chat/server/routes/ml_routes/generic_ml_router.ts [122-127] if (mlResponse.status && mlResponse.status >= 400) { - const mlClientError = new MLClientError(mlResponse.body); + const errorMessage = typeof mlResponse.body === 'string' + ? mlResponse.body + : JSON.stringify(mlResponse.body); + const mlClientError = new MLClientError(errorMessage); mlClientError.statusCode = mlResponse.status; mlClientError.statusText = mlResponse.statusText; throw mlClientError; } Suggestion importance[1-10]: 6 __ Why: If mlResponse.body is an object, passing it directly to Error constructor results in [object Object] as the message, which is unhelpful for debugging. Using JSON.stringify for non-string bodies is a valid improvement, though it depends on the MLClientError constructor accepting a message (which ties to suggestion 1).	Low
General	Add constructor with proper error initialization The MLClientError class doesn't call super() with a message or set the error name, which can make debugging harder. Add a constructor that accepts a message parameter, calls super(message), and sets this.name = 'MLClientError' to ensure proper error identification and stack traces. src/plugins/chat/server/routes/ml_routes/ml_client_error.ts [6-9] export class MLClientError extends Error { public statusCode?: number; public statusText?: string; + + constructor(message?: string) { + super(message); + this.name = 'MLClientError'; + } } Suggestion importance[1-10]: 5 __ Why: Adding a proper constructor with super(message) and this.name improves error identification and stack traces, but this is a minor improvement since the class still functions without it. The improved_code accurately reflects the suggested change.	Low

[github-actions]

Persistent review updated to latest commit a2dcd2d

[codecov]

Codecov Report

❌ Patch coverage is 0% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 61.49%. Comparing base (15cb9b0) to head (a2dcd2d).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
.../chat/server/routes/ml_routes/generic_ml_router.ts	0.00%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #11543      +/-   ##
==========================================
- Coverage   61.49%   61.49%   -0.01%     
==========================================
  Files        4988     4988              
  Lines      136627   136632       +5     
  Branches    23623    23624       +1     
==========================================
+ Hits        84022    84023       +1     
- Misses      46569    46574       +5     
+ Partials     6036     6035       -1     

Flag	Coverage Δ
Linux_1	25.05% <ø> (ø)
Linux_2	39.41% <ø> (ø)
Linux_3	42.60% <0.00%> (-0.01%)	⬇️
Linux_4	33.87% <ø> (+<0.01%)	⬆️
Windows_1	25.07% <ø> (ø)
Windows_2	39.39% <ø> (ø)
Windows_3	42.60% <0.00%> (-0.01%)	⬇️
Windows_4	33.87% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.