-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added Outlook integration toolkit #1
Conversation
1. outlookintegration.ts created 2. implement outlook api for read and send message 3. export the function to index.ts
Clone of the PR langchain-ai/langchainjs#3465 |
My review is in progress 📖 - I will have feedback for you in a few minutes! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have reviewed your code and found 4 potential issues.
To discuss my individual comments that I have added, tag me in replies using @korbit-ai.
Please react with a 👍 to my comments that you find helpful and a 👎 to those you find unhelpful - this will help me learn and improve as we collaborate.
if (!response.ok) { | ||
throw new Error(`Error: ${response.status}`); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When making requests to the Microsoft Graph API in the readEmails
and sendEmail
methods, the code is not handling potential API errors effectively. If the API returns an error response, the code throws a generic error without providing specific details about the API error. To improve this, consider checking the response status code and parsing the error response from the API. You can then throw a more specific error or create a custom error class that includes the relevant details from the API error response. This will provide more meaningful error messages to the caller of the method and help with debugging and error handling.
export class OutlookIntegration extends Tool { | ||
accessToken: string; // Store the OAuth2 access token | ||
|
||
constructor(params: ToolParams, accessToken: string) { | ||
super(params); | ||
this.accessToken = accessToken; // Initialize with an OAuth2 access token | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The OutlookIntegration class extends the Tool class but doesn't implement its required methods. To ensure proper functionality, please implement the 'name' and 'description' properties, and the 'call' method as required by the Tool class. This will prevent potential runtime errors and ensure the class behaves correctly within the LangChain framework.
export class OutlookIntegration extends Tool { | ||
accessToken: string; // Store the OAuth2 access token | ||
|
||
constructor(params: ToolParams, accessToken: string) { | ||
super(params); | ||
this.accessToken = accessToken; // Initialize with an OAuth2 access token | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security Issue: Direct usage of access token
The code directly uses the access token obtained from the user to make API requests to the Microsoft Graph API. If this access token is leaked or exposed, it could allow unauthorized access to the user's Outlook data, compromising their privacy and security.
Actionable Feedback:
Instead of directly using the access token, consider implementing a server-side proxy that securely stores and manages the access tokens. The client-side code should make requests to your own server, which then makes the API requests to the Microsoft Graph API using the stored access token. This way, the access token is not exposed in the client-side code.
async readEmails(): Promise<Email[]> { | ||
try { | ||
const response = await fetch("https://graph.microsoft.com/v1.0/me/mailFolders('Inbox')/messages?$select=sender,subject", { | ||
headers: { | ||
Authorization: `Bearer ${this.accessToken}`, | ||
}, | ||
}); | ||
|
||
if (!response.ok) { | ||
throw new Error(`Error: ${response.status}`); | ||
} | ||
|
||
const data = await response.json(); | ||
return data.value; // Assuming 'value' contains the array of emails | ||
} catch (error) { | ||
console.error("Failed to read emails:", error); | ||
throw error; | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The readEmails method currently doesn't handle pagination, which may lead to incomplete data retrieval if there are more emails than the default page size. Consider implementing pagination to ensure all emails are fetched. You can do this by checking for an '@odata.nextLink' property in the response and making additional requests until all emails are retrieved.
@coderabbitai review |
Actions performedReview triggered.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis update introduces the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant OutlookIntegration
participant MicrosoftGraphAPI
User->>OutlookIntegration: readEmails()
OutlookIntegration->>MicrosoftGraphAPI: GET /me/mailFolders/inbox/messages
MicrosoftGraphAPI-->>OutlookIntegration: Emails
OutlookIntegration-->>User: Return Emails
User->>OutlookIntegration: sendEmail(to, subject, content)
OutlookIntegration->>MicrosoftGraphAPI: POST /sendMail
MicrosoftGraphAPI-->>OutlookIntegration: Confirmation
OutlookIntegration-->>User: Success Message
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
@coderabbitai review |
Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
langchain/src/tools/outlookIntegration.ts (1)
70-70
: Consider enhancing logging for successful email sends.While logging the success message, consider including more details such as the recipient's email address for better traceability.
- console.log("Email sent successfully"); + console.log(`Email sent successfully to ${to}`);
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- langchain/src/tools/index.ts (1 hunks)
- langchain/src/tools/outlookIntegration.ts (1 hunks)
Files skipped from review due to trivial changes (1)
- langchain/src/tools/index.ts
Additional comments not posted (4)
langchain/src/tools/outlookIntegration.ts (4)
10-16
: Implement required methods and properties fromTool
.The
OutlookIntegration
class extendsTool
but does not implement the requiredname
,description
, andcall
method. Implement these to ensure the class functions correctly within the LangChain framework.
11-11
: Security Concern: Avoid direct usage of access tokens.Directly using the access token in client-side code can lead to security vulnerabilities. Consider implementing a server-side proxy to manage access tokens securely.
26-28
: Improve error handling for API requests.The current error handling is generic and does not provide detailed information about API errors. Consider parsing the error response to provide more specific error messages.
18-36
: Implement pagination for reading emails.The
readEmails
method does not handle pagination, which can lead to incomplete data retrieval. Implement pagination by checking for the@odata.nextLink
property in the response.
if (!response.ok) { | ||
throw new Error(`Error: ${response.status}`); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Enhance error handling for sending emails.
Improve the error handling by parsing the response to provide more specific error messages. This will help in debugging issues related to sending emails.
- throw new Error(`Error: ${response.status}`);
+ const errorDetails = await response.json();
+ throw new Error(`Error sending email: ${response.status} - ${errorDetails.error.message}`);
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
if (!response.ok) { | |
throw new Error(`Error: ${response.status}`); | |
} | |
if (!response.ok) { | |
const errorDetails = await response.json(); | |
throw new Error(`Error sending email: ${response.status} - ${errorDetails.error.message}`); | |
} |
/review |
PR Reviewer Guide 🔍
|
Fixes # 3005 issue
Description: Added basic tools to interact with Microsoft Graph Api that can send or read user's emails. Classes to complete the authentication flows are also added to perform user login or provide accessToken that must be used for Microsoft Graph.
Issue: langchain-ai/langchainjs#3005
Dependencies: openurl package is used during user log in
Tag maintainer: @jacoblee93 , and any of the other maintainers if needed
Hello,
This is our first PR and we hope that our changes will be helpful to the community. We have run make format, make lint and make test locally before submitting the PR. To our knowledge, our changes do not introduce any new errors.
Our PR has added basic tools to interact with Microsoft Graph Api that can send or read user's emails. Classes to complete the authentication flows are also added to perform user login or provide accessToken that must be used for Microsoft Graph. Nonetheless, we have added tests to test our changes, note the test is dependent on the necessary credentials in .env . These changes were put together by me, @oscarchen178, @Qi123123Li, and @SimonLi1020 We're a group of students from UofT submitting this for a project. Please test the code and let us know if there are any issues.
Thank you in advance to the maintainers for their time.
Description by Korbit AI
Note
This feature is in early access. You can enable or disable it in the Korbit Console.
What change is being made?
Add an Outlook integration toolkit to the codebase, including functionality to read and send emails using the Microsoft Graph API.
Why are these changes being made?
This change introduces a new tool to facilitate email operations within the application, leveraging OAuth2 for secure access. This integration is essential for users who need to manage their Outlook emails programmatically.
Summary by CodeRabbit
New Features
OutlookIntegration
allowing users to read and send emails via the Microsoft Outlook API.Enhancements
OutlookIntegration
entity for broader access within the application.