feat: update backend error #1089
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| @@ -86,5 +89,6 @@ export class BackendClient { | |||
| }); | |||
|
|
|||
| setAxiosClientConfig(axiosClient.instance); | |||
| this.instance = axiosClient.instance; | |||
There was a problem hiding this comment.
Redundant assignment of this.instance. This is already assigned on line 39. Consider removing this duplicate assignment or add a comment explaining why it's needed twice.
| const errorTypeFromBE = axiosError.response?.data?.error?.type; | ||
| const errorMessage = axiosError.response?.data?.error?.message; | ||
|
|
||
| const genericMessage = `${errorNameFromBE || predefinedError.message} ${errorTypeFromBE ? `- ${errorTypeFromBE}` : ""} on ${axiosError.config?.baseURL! + axiosError.config?.url!}`; |
There was a problem hiding this comment.
Consider adding null/undefined checks for axiosError.config?.baseURL and axiosError.config?.url. While the optional chaining helps prevent crashes, concatenating with undefined values could lead to "undefined" appearing in the error message.
|
This comment was generated by github-actions[bot]! JS SDK Coverage Report📊 Coverage report for JS SDK can be found at the following URL: 📁 Test report folder can be found at the following URL: |
| expect(e.description).toBe("Not found"); | ||
| expect(e.errorId).toBeDefined(); | ||
| expect(e.name).toBe("ComposioError"); | ||
| expect(e.possibleFix).toBe(e.possibleFix); |
There was a problem hiding this comment.
The test assertion expect(e.possibleFix).toBe(e.possibleFix) is comparing a value with itself, which will always pass. This should probably be comparing against an expected value instead.
js/src/sdk/index.spec.ts
Outdated
|
|
||
| try { | ||
| await client.apps.list(); | ||
| const apps = await client.apps.list(); | ||
| console.log(apps); |
There was a problem hiding this comment.
The console.log(apps) statement should be removed as it's not needed for the test and could clutter test output.
js/src/sdk/index.spec.ts
Outdated
| expect(error.name).toBe("ComposioError"); | ||
| expect(error.possibleFix).toBe(errorInfo.possibleFix); | ||
| if (e instanceof ComposioError) { | ||
| console.log(e); |
There was a problem hiding this comment.
Remove console.log(e) from the test case. Debug logs should not be committed to the test suite.
Code Review SummaryOverall AssessmentThe changes improve error handling by better aligning with backend error responses and adding session tracking. The code quality is generally good, but there are a few areas that need attention. Strengths
Areas for Improvement
Rating: 7/10Good improvements to error handling, but needs cleanup of debug code and stronger validation. |
himanshu-dixit
requested review from
plxity and
utkarsh-dixit
as code owners
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to c19c9bf in 36 seconds
More details
- Looked at
374lines of code in8files - Skipped
0files when reviewing. - Skipped posting
4drafted comments based on config settings.
1. js/src/sdk/index.spec.ts:77
- Draft comment:
expect(error.message).toContain("InvalidRequestError");
- Reason this comment was not posted:
Comment looked like it was already resolved.
2. js/src/sdk/index.spec.ts:78
- Draft comment:
expect(error.message).toContain("InvalidRequestError");
- Reason this comment was not posted:
Marked as duplicate.
3. js/src/sdk/index.spec.ts:121
- Draft comment:
mock.onGet("/api/v1/apps").reply(500, { detail: "Internal Server Error" });
- Reason this comment was not posted:
Comment was on unchanged code.
4. js/src/sdk/index.spec.ts:122
- Draft comment:
error: {
type: "ServerError",
name: "InternalServerError",
message: "Internal Server Error",
},
- Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_ATOOL19T6OMPQiNN
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
Important
Improved error handling in the SDK with detailed messages, added session ID to context, and updated tests for new error formats.
getAPIErrorDetails()informatter.tsto provide more detailed error messages and descriptions.BE_STATUS_CODE_TO_SDK_ERROR_CODESwithAPI_TO_SDK_ERROR_CODEinerror.tsandconstants.ts.handleAllError()andthrowAPIError()inerror.ts.sessionIdtoComposioSDKContextincomposioContext.ts.console.logstatements frombase.toolset.spec.ts.index.spec.tsto check for new error message formats.instanceproperty toBackendClientinbackendClient.tsfor Axios instance management.This description was created by
for c19c9bf. It will automatically update as commits are pushed.