feat: add custom auth data in JS SDK #758
Conversation
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to 6d4f4d1 in 21 seconds
More details
- Looked at
57lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_GL9n0pf3nXvMzU09
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.
|
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: |
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 23d7924 in 10 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
0drafted comments based on config settings.
Workflow ID: wflow_oFNm6WoCTK3cO1qm
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 68e58d1 in 25 seconds
More details
- Looked at
26lines of code in2files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. js/src/sdk/models/Entity.ts:38
- Draft comment:
The error message is too specific and assumes the user is trying to use local tools. Consider making it more general to cover other possible scenarios. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable:
The comment is addressing a change made in the diff, specifically the modification of the error message. The comment suggests that the error message might be too specific, which could be a valid point if the error message is intended to cover a broader range of scenarios. However, without additional context on the intended use cases, it's difficult to definitively say if the comment is correct.
The comment might be assuming too much about the intended use cases of the error message. The specificity could be intentional and necessary for the context in which this code is used.
The comment raises a valid point about the potential over-specificity of the error message, which could be worth considering if the error message is meant to be more general.
The comment is relevant to the change made in the diff and raises a potentially valid concern about the specificity of the error message. It should be kept for consideration.
Workflow ID: wflow_qldsQqD4CKXnkM0i
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on a45299e in 11 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. js/src/sdk/models/Entity.ts:38
- Draft comment:
Consider including a documentation link or additional guidance in the error message to help users troubleshoot the issue. - Reason this comment was not posted:
Confidence changes required:50%
The error message in line 38 was updated to remove the documentation link. This change might reduce the helpfulness of the error message for users who encounter this issue.
Workflow ID: wflow_pdkDqOpJ1G1HeUIy
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 72b8ec8 in 11 seconds
More details
- Looked at
13lines of code in1files - Skipped
0files when reviewing. - Skipped posting
1drafted comments based on config settings.
1. js/src/sdk/models/Entity.ts:38
- Draft comment:
Consider adding a period at the end of the error message for consistency with standard error message formatting.
throw new Error(`Could not find action: ${actionName}.`);
- Reason this comment was not posted:
Confidence changes required:50%
The error message in line 38 was updated to remove the period at the end. This change is unnecessary and inconsistent with standard error message formatting, which typically includes a period at the end of sentences.
Workflow ID: wflow_7eJraBWVCUWBs9GX
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
Important
Add
ParameterandCustomAuthDatatypes for custom authentication inExecuteActionData, update error handling, and remove logger warning in JS SDK.Parametertype withname,in, andvaluefields.CustomAuthDatatype withbase_url,parameters, and optionalbody.ExecuteActionDatainactions.tsto includeauthConfigfield of typeCustomAuthData.Entity.tsto include documentation link for unsupported local tools.factory.tsfor unsupported local tools in host environment.This description was created by
for 72b8ec8. It will automatically update as commits are pushed.