Closed utkarsh-dixit closed 2 months ago
β±οΈ Estimated effort to review [1-5] | 3 |
π§ͺ Relevant tests | Yes |
π Security concerns |
- Sensitive information exposure: The configuration file explicitly includes an authorization token using `process.env.API_TOKEN`. Ensure that this token is securely managed and not exposed in logs or error messages. Consider using secrets management tools for better security practices. |
β‘ Key issues to review |
Possible Bug: The use of synchronous and asynchronous exec calls within the same test step could lead to race conditions or unhandled promise rejections. Consider using async/await consistently for better error handling and control flow. |
Error Handling: The error handling in the test script could be improved by adding more specific error messages and handling specific types of exceptions more gracefully. |
Category | Suggestion | Score |
Best practice |
Add
___
**Consider removing the | 8 |
Use Playwright APIs instead of
___
**Instead of using | 7 | |
Use Playwright's built-in error handling and reporting mechanisms instead of manual try-catch blocks___ **Instead of catching and logging errors within the test, consider using Playwright'sbuilt-in error handling and reporting mechanisms to provide more structured and informative test results.** [js/tests/run-tests.spec.ts [9-42]](https://github.com/ComposioHQ/composio/pull/191/files#diff-b41ef3bc21900850abd262a5c6c563e7c38345a3b7fe580914a503895b430507R9-R42) ```diff -try { - const files = fs.readdirSync(examplesDir); - // ... -} catch (err) { - console.error(`Unable to read examples directory: ${err}`); -} +const files = await fs.promises.readdir(examplesDir); +// ... ``` Suggestion importance[1-10]: 5Why: While using built-in error handling can make the code cleaner, the suggestion incorrectly replaces synchronous file reading with an asynchronous one without adjusting the surrounding code context, which could lead to issues. | 5 | |
Enhancement |
Add a
___
**Consider adding a | 7 |
Add a
___
**It might be beneficial to add a | 7 | |
Add a
___
**Consider adding a | 7 | |
Add a script for running tests to the
___
**Consider adding a script for running tests, such as | 7 | |
Possible issue |
Add a timeout to the
___
**Add a timeout to the | 6 |
**Action:** JS tests |
**Failed stage:** [Run tests](https://github.com/ComposioHQ/composio/actions/runs/9743888574/job/26888344993) [β] |
**Failed test name:** tests/run-tests.spec.ts:13:9 βΊ e2e tests/run-tests.spec.ts:13:9 βΊ langchain tests/run-tests.spec.ts:13:9 βΊ openai |
**Failure summary:**
The action failed due to multiple errors in different tests:e2e test failed because of a BadRequestError caused by a validation error in the request payload. The data property must be an object and should not be empty.langchain test failed because the received value in the expect assertion was undefined , which is not allowed. openai test failed due to a TypeError caused by attempting to read properties of undefined (specifically, no_auth property in app.yaml ). |
Relevant error logs:```yaml 1: ##[group]Operating System 2: Ubuntu ... 649: COMPOSIO_BASE_URL: *** 650: OPENAI_API_KEY: *** 651: ##[endgroup] 652: > composio-core@0.1.4 test /home/runner/work/composio/composio/js 653: > playwright test tests/* 654: Running 3 tests using 1 worker 655: Running example: e2e 656: /home/runner/work/composio/composio/js/lib/sdk/client/core/request.js:264 657: throw new ApiError_1.ApiError(options, result, error); 658: ^ 659: ApiError: Bad Request 660: at catchErrorCodes (/home/runner/work/composio/composio/js/lib/sdk/client/core/request.js:264:15) 661: at /home/runner/work/composio/composio/js/lib/sdk/client/core/request.js:309:45 662: [90m at process.processTicksAndRejections (node:internal/process/task_queues:95:5)[39m { 663: url: [32m'***/v1/connectedAccounts'[39m, 664: status: [33m400[39m, 665: statusText: [32m'Bad Request'[39m, 666: body: { 667: message: [32m'Validation error. Please check your input.'[39m, 668: errors: [ ... 674: property: [32m'data'[39m, 675: children: [], 676: constraints: { 677: isObject: [32m'data must be an object'[39m, 678: isNotEmpty: [32m'data should not be empty'[39m 679: } 680: } 681: ], 682: stack: [32m'Error: \n'[39m + 683: [32m' at new HttpError (/app/node_modules/.pnpm/routing-controllers@0.10.4_class-transformer@0.5.1_class-validator@0.14.1/node_modules/src/http-error/HttpError.ts:16:18)\n'[39m + 684: [32m' at new BadRequestError (/app/node_modules/.pnpm/routing-controllers@0.10.4_class-transformer@0.5.1_class-validator@0.14.1/node_modules/src/http-error/BadRequestError.ts:10:5)\n'[39m + ... 690: method: [32m'POST'[39m, 691: url: [32m'/v1/connectedAccounts'[39m, 692: body: { 693: integrationId: [32m'3011084c-0c3e-4787-9949-8179675c1c5b'[39m, 694: userUuid: [32m'default'[39m, 695: redirectUri: [90mundefined[39m 696: }, 697: mediaType: [32m'application/json'[39m, 698: errors: { [32m'404'[39m: [32m'{\n "message": "Connector not found"\n}'[39m } 699: } 700: } 701: Node.js v20.15.0 702: exec error: Error: Command failed: pnpm build && cd /home/runner/work/composio/composio/js/examples/e2e && pnpm start 703: /home/runner/work/composio/composio/js/lib/sdk/client/core/request.js:264 704: throw new ApiError_1.ApiError(options, result, error); 705: ^ 706: ApiError: Bad Request 707: at catchErrorCodes (/home/runner/work/composio/composio/js/lib/sdk/client/core/request.js:264:15) 708: at /home/runner/work/composio/composio/js/lib/sdk/client/core/request.js:309:45 709: [90m at process.processTicksAndRejections (node:internal/process/task_queues:95:5)[39m { 710: url: [32m'***/v1/connectedAccounts'[39m, 711: status: [33m400[39m, 712: statusText: [32m'Bad Request'[39m, 713: body: { 714: message: [32m'Validation error. Please check your input.'[39m, 715: errors: [ ... 721: property: [32m'data'[39m, 722: children: [], 723: constraints: { 724: isObject: [32m'data must be an object'[39m, 725: isNotEmpty: [32m'data should not be empty'[39m 726: } 727: } 728: ], 729: stack: [32m'Error: \n'[39m + 730: [32m' at new HttpError (/app/node_modules/.pnpm/routing-controllers@0.10.4_class-transformer@0.5.1_class-validator@0.14.1/node_modules/src/http-error/HttpError.ts:16:18)\n'[39m + 731: [32m' at new BadRequestError (/app/node_modules/.pnpm/routing-controllers@0.10.4_class-transformer@0.5.1_class-validator@0.14.1/node_modules/src/http-error/BadRequestError.ts:10:5)\n'[39m + ... 737: method: [32m'POST'[39m, 738: url: [32m'/v1/connectedAccounts'[39m, 739: body: { 740: integrationId: [32m'3011084c-0c3e-4787-9949-8179675c1c5b'[39m, 741: userUuid: [32m'default'[39m, 742: redirectUri: [90mundefined[39m 743: }, 744: mediaType: [32m'application/json'[39m, 745: errors: { [32m'404'[39m: [32m'{\n "message": "Connector not found"\n}'[39m } 746: } 747: } 748: Node.js v20.15.0 749: FRunning example: langchain 750: BadRequestError: 400 Invalid 'functions': empty array. Expected an array with minimum length 1, but got an empty array instead. 751: at APIError.generate (file:///home/runner/work/composio/composio/js/node_modules/[4m.pnpm[24m/openai@4.51.0/node_modules/[4mopenai[24m/error.mjs:41:20) 752: at OpenAI.makeStatusError (file:///home/runner/work/composio/composio/js/node_modules/[4m.pnpm[24m/openai@4.51.0/node_modules/[4mopenai[24m/core.mjs:268:25) ... 773: [32m'x-ratelimit-limit-tokens'[39m: [32m'40000'[39m, 774: [32m'x-ratelimit-remaining-requests'[39m: [32m'4999'[39m, 775: [32m'x-ratelimit-remaining-tokens'[39m: [32m'39925'[39m, 776: [32m'x-ratelimit-reset-requests'[39m: [32m'12ms'[39m, 777: [32m'x-ratelimit-reset-tokens'[39m: [32m'112ms'[39m, 778: [32m'x-request-id'[39m: [32m'req_d4f4aec4d359971230a2b70aa337303d'[39m 779: }, 780: request_id: [32m'req_d4f4aec4d359971230a2b70aa337303d'[39m, 781: error: { 782: message: [32m"Invalid 'functions': empty array. Expected an array with minimum length 1, but got an empty array instead."[39m, 783: type: [32m'invalid_request_error'[39m, 784: param: [32m'functions'[39m, 785: code: [32m'empty_array'[39m 786: }, 787: code: [32m'empty_array'[39m, 788: param: [32m'functions'[39m, 789: type: [32m'invalid_request_error'[39m, 790: attemptNumber: [33m1[39m, 791: retriesLeft: [33m6[39m 792: } 793: stderr: undefined 794: stdout: undefined 795: exec error: Error: [2mexpect([22m[31mreceived[39m[2m).[22mtoContain[2m([22m[32mexpected[39m[2m) // indexOf[22m 796: [1mMatcher error[22m: [31mreceived[39m value must not be null nor undefined 797: Received has value: [31mundefined[39m 798: FRunning example: openai 799: /home/runner/work/composio/composio/js/lib/sdk/index.js:105 800: if (app.yaml.no_auth) { 801: ^ 802: TypeError: Cannot read properties of undefined (reading 'no_auth') 803: at Entity.execute (/home/runner/work/composio/composio/js/lib/sdk/index.js:105:22) 804: [90m at process.processTicksAndRejections (node:internal/process/task_queues:95:5)[39m 805: at async OpenAIToolSet.execute_tool_call (/home/runner/work/composio/composio/js/lib/frameworks/openai.js:55:31) 806: at async OpenAIToolSet.handle_tool_call (/home/runner/work/composio/composio/js/lib/frameworks/openai.js:61:30) 807: at async executeAgent [90m(file:///home/runner/work/composio/composio/js/examples/openai/[39mdemo.mjs:41:5[90m)[39m 808: Node.js v20.15.0 809: exec error: Error: Command failed: pnpm build && cd /home/runner/work/composio/composio/js/examples/openai && pnpm start 810: /home/runner/work/composio/composio/js/lib/sdk/index.js:105 811: if (app.yaml.no_auth) { 812: ^ 813: TypeError: Cannot read properties of undefined (reading 'no_auth') 814: at Entity.execute (/home/runner/work/composio/composio/js/lib/sdk/index.js:105:22) 815: [90m at process.processTicksAndRejections (node:internal/process/task_queues:95:5)[39m 816: at async OpenAIToolSet.execute_tool_call (/home/runner/work/composio/composio/js/lib/frameworks/openai.js:55:31) 817: at async OpenAIToolSet.handle_tool_call (/home/runner/work/composio/composio/js/lib/frameworks/openai.js:61:30) 818: at async executeAgent [90m(file:///home/runner/work/composio/composio/js/examples/openai/[39mdemo.mjs:41:5[90m)[39m 819: Node.js v20.15.0 820: F 821: 1) tests/run-tests.spec.ts:13:9 βΊ e2e ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ 822: Error: Command failed: pnpm build && cd /home/runner/work/composio/composio/js/examples/e2e && pnpm start 823: /home/runner/work/composio/composio/js/lib/sdk/client/core/request.js:264 824: throw new ApiError_1.ApiError(options, result, error); 825: ^ 826: ApiError: Bad Request 827: at lib/sdk/client/core/request.js:264 828: 262 | const error = errors[result.status]; 829: 263 | if (error) { 830: > 264 | throw new ApiError_1.ApiError(options, result, error); 831: | ^ 832: 265 | } 833: 266 | if (!result.ok) { 834: 267 | const errorStatus = (_a = result.status) !== null && _a !== void 0 ? _a : 'unknown'; 835: at catchErrorCodes (/home/runner/work/composio/composio/js/lib/sdk/client/core/request.js:264:15) 836: at /home/runner/work/composio/composio/js/lib/sdk/client/core/request.js:309:45 837: [90m at process.processTicksAndRejections (node:internal/process/task_queues:95:5)[39m { 838: url: [32m'***/v1/connectedAccounts'[39m, 839: status: [33m400[39m, 840: statusText: [32m'Bad Request'[39m, 841: body: { 842: message: [32m'Validation error. Please check your input.'[39m, 843: errors: [ ... 849: property: [32m'data'[39m, 850: children: [], 851: constraints: { 852: isObject: [32m'data must be an object'[39m, 853: isNotEmpty: [32m'data should not be empty'[39m 854: } 855: } 856: ], 857: stack: [32m'Error: \n'[39m + 858: [32m' at new HttpError (/app/node_modules/.pnpm/routing-controllers@0.10.4_class-transformer@0.5.1_class-validator@0.14.1/node_modules/src/http-error/HttpError.ts:16:18)\n'[39m + 859: [32m' at new BadRequestError (/app/node_modules/.pnpm/routing-controllers@0.10.4_class-transformer@0.5.1_class-validator@0.14.1/node_modules/src/http-error/BadRequestError.ts:10:5)\n'[39m + ... 865: method: [32m'POST'[39m, 866: url: [32m'/v1/connectedAccounts'[39m, 867: body: { 868: integrationId: [32m'3011084c-0c3e-4787-9949-8179675c1c5b'[39m, 869: userUuid: [32m'default'[39m, 870: redirectUri: [90mundefined[39m 871: }, 872: mediaType: [32m'application/json'[39m, 873: errors: { [32m'404'[39m: [32m'{\n "message": "Connector not found"\n}'[39m } 874: } 875: } 876: Node.js v20.15.0 877: at catchErrorCodes (/home/runner/work/composio/composio/js/lib/sdk/client/core/request.js:264:15) 878: at /home/runner/work/composio/composio/js/lib/sdk/client/core/request.js:309:45 879: at /home/runner/work/composio/composio/js/tests/run-tests.spec.ts:18:46 880: at /home/runner/work/composio/composio/js/tests/run-tests.spec.ts:14:13 881: 2) tests/run-tests.spec.ts:13:9 βΊ langchain ββββββββββββββββββββββββββββββββββββββββββββββββββββββ 882: Error: [2mexpect([22m[31mreceived[39m[2m).[22mtoContain[2m([22m[32mexpected[39m[2m) // indexOf[22m 883: [1mMatcher error[22m: [31mreceived[39m value must not be null nor undefined 884: Received has value: [31mundefined[39m 885: 21 | 886: 22 | // Assert some stuff on stdout for test checks 887: > 23 | expect(stdout).toContain('Expected output'); 888: | ^ 889: 24 | expect(stderr).toBe(''); 890: 25 | resolve(); 891: 26 | } catch (error) { 892: at /home/runner/work/composio/composio/js/tests/run-tests.spec.ts:23:26 893: at /home/runner/work/composio/composio/js/tests/run-tests.spec.ts:14:13 894: 3) tests/run-tests.spec.ts:13:9 βΊ openai βββββββββββββββββββββββββββββββββββββββββββββββββββββββββ 895: Error: Command failed: pnpm build && cd /home/runner/work/composio/composio/js/examples/openai && pnpm start 896: /home/runner/work/composio/composio/js/lib/sdk/index.js:105 897: if (app.yaml.no_auth) { 898: ^ 899: TypeError: Cannot read properties of undefined (reading 'no_auth') ... 911: at async OpenAIToolSet.handle_tool_call (/home/runner/work/composio/composio/js/lib/frameworks/openai.js:61:30) 912: at async executeAgent [90m(file:///home/runner/work/composio/composio/js/examples/openai/[39mdemo.mjs:41:5[90m)[39m 913: Node.js v20.15.0 914: at Entity.execute (/home/runner/work/composio/composio/js/lib/sdk/index.js:105:22) 915: at async OpenAIToolSet.execute_tool_call (/home/runner/work/composio/composio/js/lib/frameworks/openai.js:55:31) 916: at async OpenAIToolSet.handle_tool_call (/home/runner/work/composio/composio/js/lib/frameworks/openai.js:61:30) 917: at /home/runner/work/composio/composio/js/tests/run-tests.spec.ts:18:46 918: at /home/runner/work/composio/composio/js/tests/run-tests.spec.ts:14:13 919: 3 failed 920: tests/run-tests.spec.ts:13:9 βΊ e2e βββββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ 921: tests/run-tests.spec.ts:13:9 βΊ langchain βββββββββββββββββββββββββββββββββββββββββββββββββββββββ 922: tests/run-tests.spec.ts:13:9 βΊ openai ββββββββββββββββββββββββββββββββββββββββββββββββββββββββββ 923: ELIFECYCLEβ Test failed. See above for more details. 924: ##[error]Process completed with exit code 1. ``` |
This PR includes several updates aimed at enhancing the JavaScript examples, testing workflow, and incorporating the COMPOSIO_API_KEY
and COMPOSIO_BASE_URL
in the project. Below are the key changes:
Added Example Tests for JavaScript and Updated Existing Examples:
Updated GitHub Actions Workflow:
.github/workflows/common.yml
for running JavaScript tests automatically.Modified SDK Configuration:
OpenAPI
configuration and the Composio
class to support COMPOSIO_API_KEY
and COMPOSIO_BASE_URL
environment variables.Included Playwright for Testing:
General Code Maintenance:
This PR mainly affects multiple areas including JavaScript examples, workflows, and test configurations. The important files are:
.github/workflows/common.yml
js/examples/e2e/demo.mjs
js/examples/langchain/demo.mjs
js/examples/openai/demo.mjs
js/package.json
js/playwright.config.ts
js/tests/run-tests.spec.ts
js/src/sdk/client/core/OpenAPI.ts
js/src/sdk/index.ts
PR Type
Tests, Enhancement
Description
package.json
to include Playwright as a dependency and to run Playwright tests.Changes walkthrough π
run-tests.spec.ts
Add Playwright tests for example projects
js/tests/run-tests.spec.ts
.last-run.json
Add file to store last test run results
js/test-results/.last-run.json - Added file to store the results of the last test run.
playwright.config.ts
Configure Playwright settings for testing
js/playwright.config.ts
package.json
Update package.json to include Playwright tests
js/package.json
package.json
Add start script for e2e example
js/examples/e2e/package.json - Added start script to run demo.mjs.
package.json
Add start script for OpenAI example
js/examples/openai/package.json - Added start script to run demo.mjs.
package.json
Add start script for LangChain example
js/examples/langchain/package.json - Added start script to run demo.mjs.