feat: enhance type and errors #812
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Hey @himanshu-dixit, here is an example of how you can ask me to improve this pull request: @Sweep Add unit tests for the new `CEG` (Composio Error Generator) class in `error.ts` to verify: 📖 For more information on how to use Sweep, please read our documentation. |
|
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: |
![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! Incremental review on e0b02ca in 18 seconds
More details
- Looked at
16lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. js/src/index.ts:8
- Draft comment:
Consider usingimportinstead ofrequirefor consistency with other import statements.
import { APPS, ACTIONS } from "./constants";
- Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_0DwoLWjlaSzG5CAI
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to febe169 in 1 minute and 17 seconds
More details
- Looked at
659lines of code in14files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. js/src/sdk/actionRegistry.spec.ts:131
- Draft comment:
The methodgetToolsis used but not defined in theActionRegistryclass. Ensure that this method is implemented or correct the method name if it's a typo. - Reason this comment was not posted:
Marked as duplicate.
2. js/src/sdk/base.toolset.ts:209
- Draft comment:
The methodgetToolsis used but not defined in theComposioToolSetclass. Ensure that this method is implemented or correct the method name if it's a typo. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment is incorrect because thegetToolsmethod is defined in the class. The comment suggests a missing method, but the method is present, so no code change is required.
I might be missing some context about how the method is used elsewhere, but based on the file content, the method is defined, so the comment is not valid.
The presence of the method definition in the file is sufficient evidence to conclude that the comment is incorrect.
Delete the comment because it incorrectly states that thegetToolsmethod is not defined, while it is actually defined in the file.
Workflow ID: wflow_REnxkLGTGo7u8d5F
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
js/src/sdk/actionRegistry.spec.ts
Outdated
| @@ -122,13 +122,13 @@ describe("ActionRegistry", () => { | |||
|
|
|||
| await actionRegistry.createAction(options); | |||
|
|
|||
| const actions = await actionRegistry.getActions({actions: ["testAction"]}); | |||
| const actions = await actionRegistry.getTools({actions: ["testAction"]}); | |||
There was a problem hiding this comment.
The method getTools is used but not defined in the ActionRegistry class. Ensure that this method is implemented or correct the method name if it's a typo.
There was a problem hiding this comment.
❌ Changes requested. Incremental review on 632ddba in 58 seconds
More details
- Looked at
287lines of code in9files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. js/src/sdk/models/activeTriggers.ts:47
- Draft comment:
Consider validating the response structure before casting it toTActiveTriggerto avoid potential runtime errors. - Reason this comment was not posted:
Marked as duplicate.
Workflow ID: wflow_uRiNLVDvJrtHUhIt
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| try { | ||
| const {data: response} = await apiClient.triggers.getActiveTriggers({ query: data }) | ||
|
|
||
| const newResponse = response as TActiveTriggersListResponse; |
There was a problem hiding this comment.
Consider validating the response structure before casting it to TActiveTriggersListResponse to avoid potential runtime errors.
| async create(data: InitiateConnectionPayloadDto) { | ||
| try { | ||
| const {data: res} = await apiClient.connections.initiateConnection({ body: data }); | ||
| //@ts-ignore |
There was a problem hiding this comment.
Avoid using @ts-ignore. Address the TypeScript error directly to ensure type safety.
| //@ts-ignore |
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on bb215eb in 17 seconds
More details
- Looked at
132lines of code in2files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. js/src/sdk/models/Entity.ts:135
- Draft comment:
Consider usingComposioErrorfor more informative error messages instead ofError. This applies to other instances in this file as well. - Reason this comment was not posted:
Confidence changes required:50%
The error handling is consistent across methods, but the error messages could be more informative.
Workflow ID: wflow_eZXMqR2uqeMl2CSi
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
js/src/sdk/models/activeTriggers.ts
Outdated
| createdAt: string; | ||
| updatedAt: string; | ||
| disabledAt: string | null; | ||
| disabledReason: string | null; |
There was a problem hiding this comment.
Not important, can we fix the spacing here?
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on d26f824 in 34 seconds
More details
- Looked at
20lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. js/src/sdk/utils/error.ts:113
- Draft comment:
The 401 status code should map toERROR.BACKEND.UNAUTHORIZEDinstead ofERROR.COMMON.API_KEY_UNAVAILABLE. This ensures consistency with the error definitions. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_jU3gheOL7YbY5MtW
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
Important
Enhances error handling and type definitions across the SDK, introducing
ComposioErrorandCEGfor standardized error management.ComposioErrorandCEGclasses inerror.tsfor standardized error handling.apps.ts,integrations.ts,triggers.ts, andconnectedAccounts.tsto useCEG.handleError().activeTriggers.ts,apps.ts,connectedAccounts.ts, andintegrations.tsto use specific types fromtypes.gen.Entity.tsandComposio.ts.getActionswithgetToolsinactionRegistry.spec.tsandbase.toolset.ts.clientconfiguration inservices.gen.tstothrowOnError: true.This description was created by
for d26f824. It will automatically update as commits are pushed.