-
Notifications
You must be signed in to change notification settings - Fork 11
Batch Sending Api #63
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
Conversation
…, bulk, and sandbox emails in batch
WalkthroughThe changes introduce batch email sending functionality to the codebase. This includes a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant MailtrapClient
participant MailtrapAPI
User->>MailtrapClient: batchSend(BatchSendRequest)
MailtrapClient->>MailtrapAPI: POST /api/batch (payload with base + requests)
MailtrapAPI-->>MailtrapClient: BatchSendResponse (message_ids)
MailtrapClient-->>User: Promise resolves with BatchSendResponse
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 3
🧹 Nitpick comments (10)
README.md (1)
69-74
: The new Batch Sending API section looks good, but fix the list indentation.The addition of the Batch Sending API section with links to examples aligns perfectly with the PR objectives. However, there's a minor formatting issue with the list items.
Fix the markdown list indentation by removing the leading space before each dash:
### Batch Sending API - - [Send a batch of emails](examples/batch/send-batch.ts) - - [Send a batch of transactional emails](examples/batch/transactional-batch.ts) - - [Send a batch of bulk emails](examples/batch/bulk-batch.ts) - - [Send a batch of sandbox emails](examples/batch/sandbox-batch.ts) + - [Send a batch of emails](examples/batch/send-batch.ts) + - [Send a batch of transactional emails](examples/batch/transactional-batch.ts) + - [Send a batch of bulk emails](examples/batch/bulk-batch.ts) + - [Send a batch of sandbox emails](examples/batch/sandbox-batch.ts)🧰 Tools
🪛 LanguageTool
[grammar] ~72-~72: Possible verb agreement error. Did you mean “sends”? (Some collective nouns can be treated as both singular and plural, so ‘Send’ is not always incorrect.)
Context: ...ails](examples/batch/send-batch.ts) - [Send a batch of transactional emails](exampl...(COLLECTIVE_NOUN_VERB_AGREEMENT_VBP)
🪛 markdownlint-cli2 (0.17.2)
71-71: Unordered list indentation
Expected: 0; Actual: 1(MD007, ul-indent)
72-72: Unordered list indentation
Expected: 0; Actual: 1(MD007, ul-indent)
73-73: Unordered list indentation
Expected: 0; Actual: 1(MD007, ul-indent)
74-74: Unordered list indentation
Expected: 0; Actual: 1(MD007, ul-indent)
examples/testing/send-mail.ts (1)
6-7
: Redundant documentation links - consider consolidating.There are two @see links pointing to the same article, with the second one just adding a fragment identifier to the URL.
Consider consolidating these links or making the purpose of the second link more explicit:
* @see https://help.mailtrap.io/article/69-sending-domain-setup -* @see https://help.mailtrap.io/article/69-sending-domain-setup#Demo-Domain--oYOU5" +* @see https://help.mailtrap.io/article/69-sending-domain-setup#Demo-Domain--oYOU5 "Demo Domain Setup Details"Alternatively, remove the first link if the second one with the fragment identifier is more specific and useful.
examples/batch/template.ts (1)
1-39
: Well-structured example for template-based batch sending.This example clearly demonstrates how to use the new batch sending functionality with templates. The code is well-organized with proper comments explaining prerequisites and an intuitive structure separating base properties from individual request properties.
Consider enhancing the example by adding a comment showing what the expected response structure would look like, to help users better understand the API:
}) - .then(console.log) + .then((response) => { + // Example response structure: + // { + // success: true, + // message_ids: ["msg_1234abcd", "msg_5678efgh"] + // } + console.log(response); + }) .catch(console.error);examples/batch/bulk.ts (1)
20-21
: Subject line doesn't match example typeThe subject line contains "Sandbox Email" but this is a bulk email example. This could be confusing to users.
- subject: "Sandbox Email", + subject: "Bulk Email",src/lib/MailtrapClient.ts (2)
147-149
: Consider simplifying the conditional URL constructionThe variable name
ifSandbox
is not very descriptive, and the conditional logic could be simplified for better readability.- const ifSandbox = - this.sandbox && this.testInboxId ? `/${this.testInboxId}` : ""; - const url = `${host}/api/batch${ifSandbox}`; + const testInboxPath = this.sandbox && this.testInboxId ? `/${this.testInboxId}` : ""; + const url = `${host}/api/batch${testInboxPath}`;
152-157
: Consider handling optional fields more elegantlyThe current implementation includes all fields in the mapped requests, even if they're undefined. You might want to only include fields that are actually present in the request.
- const preparedRequests = request.requests.map((req) => ({ - to: req.to, - cc: req.cc, - bcc: req.bcc, - custom_variables: req.custom_variables, - })); + const preparedRequests = request.requests.map((req) => { + const mappedRequest: Record<string, any> = { to: req.to }; + + if (req.cc) mappedRequest.cc = req.cc; + if (req.bcc) mappedRequest.bcc = req.bcc; + if (req.custom_variables) mappedRequest.custom_variables = req.custom_variables; + + return mappedRequest; + });src/types/mailtrap.ts (2)
72-77
: Consider reusing the existingSendResponse
type.The new
BatchSendResponse
type has the same structure as the existingSendResponse
type. To avoid duplication and ensure consistency, consider reusingSendResponse
instead of creating a new type.-export type BatchSendResponse = { - success: true; - message_ids: string[]; -}; +export type BatchSendResponse = SendResponse;
87-101
: Consider using union types to better represent content vs template emails.The
base
object combines properties for both content-based emails (text
,html
) and template-based emails (template_uuid
). Consider modeling this similar to the existing pattern whereexport type BatchSendRequest = { - base: { - from: Address; - subject?: string; - text?: string | Buffer; - html?: string | Buffer; - template_uuid?: string; - category?: string; - attachments?: Attachment[]; - headers?: MailtrapHeaders; - custom_variables?: CustomVariables; - reply_to?: Address; - }; + base: { + from: Address; + category?: string; + attachments?: Attachment[]; + headers?: MailtrapHeaders; + custom_variables?: CustomVariables; + reply_to?: Address; + } & ( + { + subject: string; + text?: string | Buffer; + html?: string | Buffer; + } | { + template_uuid: string; + } + ); requests: BatchSendRequestItem[]; };src/__tests__/lib/mailtrap-client.test.ts (2)
521-538
: Enhance test coverage with template variables.The test data for template-based batch sending doesn't exercise the
template_variables
property withinBatchSendRequestItem
. Add a test with different template variables for each recipient to ensure this functionality works correctly.requests: [ { to: [ { email: "[email protected]", name: "recipient1", }, ], + template_variables: { + user_name: "John Doe", + product_name: "Product A" + } }, { to: [ { email: "[email protected]", name: "recipient2", }, ], + template_variables: { + user_name: "Jane Smith", + product_name: "Product B" + } }, ],
386-691
: Consider refactoring tests to reduce duplication.The batch sending tests contain significant duplication in test data and assertion patterns. Consider extracting common setup and test data into helper functions or variables to improve maintainability.
For example, you could create a helper function for batch test setup:
function setupBatchTest(clientConfig, endpoint) { const client = new MailtrapClient(clientConfig); const expectedResponseData = { success: true, message_ids: ["0c7fd939-02cf-11ed-88c2-0a58a9feac02"], }; mock.onPost(endpoint).reply(200, expectedResponseData); const batchData = { base: { from: { email: "[email protected]", name: "Mailtrap", }, subject: "Batch Subject", text: "Batch Text", }, requests: [ { to: [ { email: "[email protected]", name: "recipient1", }, ], }, { to: [ { email: "[email protected]", name: "recipient2", }, ], }, ], }; return { client, expectedResponseData, batchData }; }Then use it in your tests:
it("successfully sends a batch of emails in bulk mode", async () => { const { client, expectedResponseData, batchData } = setupBatchTest( { token: "MY_API_TOKEN", bulk: true }, `${BULK_ENDPOINT}/api/batch` ); const result = await client.batchSend(batchData); expect(mock.history.post[0].url).toEqual(`${BULK_ENDPOINT}/api/batch`); expect(mock.history.post[0].data).toEqual( JSON.stringify({ base: batchData.base, requests: batchData.requests.map((req) => ({ to: req.to, })), }) ); expect(result).toEqual(expectedResponseData); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (10)
README.md
(1 hunks)examples/batch/bulk.ts
(1 hunks)examples/batch/sandbox.ts
(1 hunks)examples/batch/template.ts
(1 hunks)examples/batch/transactional.ts
(1 hunks)examples/testing/send-mail.ts
(1 hunks)src/__tests__/lib/mailtrap-client.test.ts
(1 hunks)src/lib/MailtrapClient.ts
(2 hunks)src/lib/mail-buffer-encoder.ts
(1 hunks)src/types/mailtrap.ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/lib/MailtrapClient.ts (2)
src/types/mailtrap.ts (2)
BatchSendRequest
(87-101)BatchSendResponse
(74-77)src/lib/mail-buffer-encoder.ts (1)
encodeMailBuffers
(8-33)
src/lib/mail-buffer-encoder.ts (1)
src/types/mailtrap.ts (1)
src/__tests__/lib/mailtrap-client.test.ts (1)
src/lib/MailtrapError.ts (1)
MailtrapError
(1-1)
🪛 LanguageTool
README.md
[grammar] ~72-~72: Possible verb agreement error. Did you mean “sends”? (Some collective nouns can be treated as both singular and plural, so ‘Send’ is not always incorrect.)
Context: ...ails](examples/batch/send-batch.ts) - [Send a batch of transactional emails](exampl...
(COLLECTIVE_NOUN_VERB_AGREEMENT_VBP)
🪛 markdownlint-cli2 (0.17.2)
README.md
71-71: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
72-72: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
73-73: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
74-74: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
🔇 Additional comments (10)
src/lib/mail-buffer-encoder.ts (1)
8-8
: Good improvement to function signature flexibility.Changing the function signature to accept
Partial<Mail>
instead of justThis change aligns well with the batch sending functionality being added in this PR and follows good TypeScript practices.
examples/batch/bulk.ts (1)
1-40
: Overall, the example looks goodThis example clearly demonstrates how to use the batch sending API in bulk mode. The code structure is clean and includes proper error handling with promise chaining.
examples/batch/transactional.ts (1)
1-38
: Example looks goodThis transactional batch sending example is well-structured and demonstrates the API usage clearly. It includes proper documentation, placeholder values, and error handling.
examples/batch/sandbox.ts (1)
1-43
: Example looks goodThe sandbox mode batch sending example is well-implemented with all the necessary configuration options. The code is clean and follows the same pattern as the other examples, ensuring consistency across the codebase.
src/lib/MailtrapClient.ts (2)
14-20
: Imports look goodThe updated import statement correctly includes the new batch-related types that are used in the implementation.
139-163
: Implementation is solidThe
batchSend
method implementation looks good. It:
- Properly determines the host based on the client configuration
- Correctly constructs the API URL with conditional test inbox ID for sandbox mode
- Uses the existing
encodeMailBuffers
function to process the base mail- Maps request items to include only the necessary fields
- Makes a POST request to the batch API endpoint with properly structured data
The implementation follows the same patterns as the existing code, which ensures consistency.
src/types/mailtrap.ts (1)
79-85
: LGTM: Clear and well-structured recipient details type.The
BatchSendRequestItem
type is well-defined with appropriate optional fields for customizing individual recipients in a batch.src/__tests__/lib/mailtrap-client.test.ts (3)
555-607
: LGTM: Comprehensive test for transactional batch sending.This test thoroughly verifies the functionality of transactional batch sending, covering the proper endpoint construction, request payload formatting, and response handling.
609-652
: LGTM: Proper API error handling test.This test effectively validates that API errors are properly caught and converted to
MailtrapError
instances with the correct error message.
654-690
: LGTM: Proper HTTP transport error handling test.This test ensures that HTTP transport errors are correctly handled and converted to
MailtrapError
instances with appropriate error messages.
…ne/template structures
… in batch sending
@coderabbitai check |
Tip For best results, initiate chat on the files or code changes. @narekhovhannisyan I've checked the PR implementation and everything looks good! The batch sending API is well-implemented with:
The approach of having a base email template with individual recipient customizations is a solid design pattern for batch operations. The implementation aligns well with the existing codebase structure while extending functionality in a useful way. |
}; | ||
|
||
try { | ||
await batchClient.batchSend(batchData); |
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.
Should we raise incompatibility error much earlier, during initialization of MailtrapClient
?
const expectedResponseData = { | ||
success: true, | ||
message_ids: ["0c7fd939-02cf-11ed-88c2-0a58a9feac02"], | ||
}; |
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.
This is not a correct schema of the Batch API response, please check https://api-docs.mailtrap.io/docs/mailtrap-api-docs/f96000c8de409-batch-send-email-including-templates
const expectedResponseData = { | ||
success: true, | ||
message_ids: ["0c7fd939-02cf-11ed-88c2-0a58a9feac02"], | ||
}; |
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 actual response schema: https://api-docs.mailtrap.io/docs/mailtrap-api-docs/48b526aa487b1-batch-send-email-including-templates
const responseData = { | ||
success: false, | ||
errors: ["from is required", "subject is required"], | ||
}; |
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.
This too doesn't match the way error response is returned from the Batch API
src/types/mailtrap.ts
Outdated
} | ||
|
||
export interface BatchSendRequest { | ||
base: InlineBatchSendBase | TemplateBatchSendBase; |
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.
base
is not required, so I guess it should be base?
?
to: BaseAddress[]; | ||
cc?: BaseAddress[]; | ||
bcc?: BaseAddress[]; | ||
custom_variables?: Record<string, string>; |
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.
There's much more supported attributes on this level too, e.g. reply_to
, text
, html
, category
..., even template_uuid
.
From API docs:
base
- General properties of all emails in the batch. Each of them can be overridden in requests for individual emails.
requests
- The list of emails. Each of them requires recipients (one of to, cc, or bcc). Each email inherits properties from base but can override them.
src/types/mailtrap.ts
Outdated
template_uuid: string; // Required for template usage | ||
template_variables?: Record<string, string>; | ||
custom_variables?: Record<string, string>; | ||
category?: string; |
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 think category
is actually not allowed when using a template (since category is set on template already)
Co-authored-by: Marcin Klocek <[email protected]>
Co-authored-by: Marcin Klocek <[email protected]>
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
♻️ Duplicate comments (2)
src/__tests__/lib/mailtrap-client.test.ts (2)
376-379
:⚠️ Potential issueUse correct API response schema
The API response schema used in the test doesn't match the actual Batch API response schema as mentioned in a previous review comment.
const expectedResponseData = { success: true, - message_ids: ["0c7fd939-02cf-11ed-88c2-0a58a9feac02"], + // Update with the actual response schema from https://api-docs.mailtrap.io/docs/mailtrap-api-docs/f96000c8de409-batch-send-email-including-templates + message_id: "0c7fd939-02cf-11ed-88c2-0a58a9feac02" };
432-435
:⚠️ Potential issueUse correct API response schema for sandbox mode
The API response schema used in the test doesn't match the actual Batch API response schema as mentioned in a previous review comment.
const expectedResponseData = { success: true, - message_ids: ["0c7fd939-02cf-11ed-88c2-0a58a9feac02"], + // Update with the actual response schema from https://api-docs.mailtrap.io/docs/mailtrap-api-docs/48b526aa487b1-batch-send-email-including-templates + message_id: "0c7fd939-02cf-11ed-88c2-0a58a9feac02" };
🧹 Nitpick comments (2)
README.md (1)
71-74
: Fix list indentation in markdownThe markdownlint static analysis tool has flagged an indentation issue with the unordered list items.
- - [Send a batch of transactional emails](examples/batch/transactional.ts) - - [Send a batch of bulk emails](examples/batch/bulk.ts) - - [Send a batch of sandbox emails](examples/batch/sandbox.ts) - - [Send a batch of emails from template](examples/batch/template.ts) + - [Send a batch of transactional emails](examples/batch/transactional.ts) + - [Send a batch of bulk emails](examples/batch/bulk.ts) + - [Send a batch of sandbox emails](examples/batch/sandbox.ts) + - [Send a batch of emails from template](examples/batch/template.ts)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
71-71: Unordered list indentation
Expected: 0; Actual: 1(MD007, ul-indent)
72-72: Unordered list indentation
Expected: 0; Actual: 1(MD007, ul-indent)
73-73: Unordered list indentation
Expected: 0; Actual: 1(MD007, ul-indent)
74-74: Unordered list indentation
Expected: 0; Actual: 1(MD007, ul-indent)
src/__tests__/lib/mailtrap-client.test.ts (1)
361-361
: Consider raising incompatibility error earlierAs mentioned in a previous review comment, the incompatibility between bulk and sandbox modes could potentially be detected during client initialization rather than when calling the
batchSend
method.This approach would provide earlier feedback to developers about the configuration issue before they attempt to use the API.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (4)
README.md
(1 hunks)examples/batch/bulk.ts
(1 hunks)src/__tests__/lib/mailtrap-client.test.ts
(1 hunks)src/types/mailtrap.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- examples/batch/bulk.ts
- src/types/mailtrap.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/__tests__/lib/mailtrap-client.test.ts (1)
src/lib/MailtrapError.ts (1)
MailtrapError
(1-1)
🪛 markdownlint-cli2 (0.17.2)
README.md
71-71: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
72-72: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
73-73: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
74-74: Unordered list indentation
Expected: 0; Actual: 1
(MD007, ul-indent)
🔇 Additional comments (9)
README.md (1)
69-75
:✅ Verification successful
Well structured documentation addition for Batch Sending API
The new section clearly documents the batch sending capabilities with appropriate links to example files, making it easier for users to understand and implement this functionality.
Let's verify the file paths and ensure they match the actual repository structure:
🏁 Script executed:
#!/bin/bash # Check if the example files exist at the specified paths echo "Verifying file paths for batch sending examples..." for file in examples/batch/{transactional,bulk,sandbox,template}.ts; do if [[ -f "$file" ]]; then echo "✅ File exists: $file" else echo "❌ File missing: $file" fi doneLength of output: 914
Approve documentation update for Batch Sending API
All example links correctly point to existing files in
examples/batch
:
- ✅ examples/batch/transactional.ts
- ✅ examples/batch/bulk.ts
- ✅ examples/batch/sandbox.ts
- ✅ examples/batch/template.ts
The added section is clear and makes it easy for users to find and run batch-sending examples.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
71-71: Unordered list indentation
Expected: 0; Actual: 1(MD007, ul-indent)
72-72: Unordered list indentation
Expected: 0; Actual: 1(MD007, ul-indent)
73-73: Unordered list indentation
Expected: 0; Actual: 1(MD007, ul-indent)
74-74: Unordered list indentation
Expected: 0; Actual: 1(MD007, ul-indent)
src/__tests__/lib/mailtrap-client.test.ts (8)
331-674
: Comprehensive test coverage for new batch sending functionalityThe test suite is well-structured and covers all the key scenarios for the batch sending functionality. It tests various modes (bulk, sandbox, transactional), template usage, and error handling.
332-368
: Good validation of incompatible modesThis test correctly verifies that batch sending rejects with a MailtrapError when both bulk and sandbox modes are enabled simultaneously.
370-423
: Bulk mode batch sending tests are correctly implementedThe test properly validates the endpoint, request payload structure, and response handling for bulk mode batch sending.
425-479
: Sandbox mode batch sending tests are correctly implementedThe test properly validates the endpoint, request payload structure, and response handling for sandbox mode batch sending.
481-536
: Template batch sending tests are correctly implementedThe test appropriately validates the batch sending functionality with templates, including the template_uuid and template_variables in the base payload.
538-590
: Transactional batch sending tests are correctly implementedThe test properly validates the endpoint, request payload structure, and response handling for transactional batch sending.
592-635
: API error handling tests are comprehensiveThe test properly verifies error handling when the API returns error messages.
637-673
: HTTP transport error handling tests are comprehensiveThe test correctly verifies that HTTP transport errors are properly handled and wrapped in a MailtrapError with an appropriate message.
…ure with detailed message IDs
…d template contexts
…to' field for improved structure
…e error handling for batch sending
…uccess responses and optional message_ids and errors
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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/__tests__/lib/mailtrap-client.test.ts
(1 hunks)src/lib/MailtrapClient.ts
(2 hunks)src/types/mailtrap.ts
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/MailtrapClient.ts
- src/types/mailtrap.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/__tests__/lib/mailtrap-client.test.ts (1)
src/lib/MailtrapError.ts (1)
MailtrapError
(1-1)
🔇 Additional comments (4)
src/__tests__/lib/mailtrap-client.test.ts (4)
331-657
: Comprehensive test coverage for batch sending functionality looks excellent.The batch sending test suite thoroughly covers all critical scenarios including mode validation, successful operations across different client configurations, template support, and error handling. The test structure follows established patterns and properly validates both request formatting and response handling.
332-368
: Mode incompatibility validation is properly tested.The test correctly verifies that using both
bulk
andsandbox
modes together throws the appropriateMailtrapError
with the expected message. This aligns with the existing incompatibility checks in the regularsend()
method.
419-425
: Request payload transformation is correctly implemented.The tests properly verify that the batch request payload is transformed to include only the
to
field from each request while preserving thebase
payload structure. This matches the expected Batch API format and demonstrates proper data sanitization.Also applies to: 480-486, 542-548, 605-611
622-630
:❓ Verification inconclusive
Review error handling test scenario for batch operations.
This test simulates a scenario where the batch request succeeds (200 status) but individual emails within the batch fail. The test expects individual response errors as string arrays. While this approach makes sense for batch operations, ensure this error format aligns with how the actual Batch API returns individual response errors.
The test assumes errors within batch responses are returned as string arrays. Please verify this matches the actual API behavior:
Also applies to: 651-655
🌐 Web query:
What is the correct error response format for individual failed emails in Mailtrap Batch API responses according to the documentation?
💡 Result:
The Mailtrap API for batch email sending provides a specific way to handle and check for errors in individual emails. When using the batch send email endpoint, users should examine both the success and error fields for each message in the response[1].
The response format maintains the same order as the original messages that were submitted in the batch, making it easier to track which emails succeeded or failed[1]. This organized format allows developers to efficiently identify problematic messages without losing track of the sequence.
Error Response Details
When an email fails to send through Mailtrap's batch API, the error information is presented in a standardized format. The API returns proper error codes along with descriptive messages to help troubleshoot the issue.
Common error response codes you might encounter include:
401 Unauthorized: This indicates authentication problems, which could happen if you're using incorrect API credentials, testing credentials instead of sending credentials, or if your custom token lacks proper permissions[3].
4xx and 5xx codes: Various other error codes provide specific information about why an email failed to send. For example:
- 450: Mailbox unavailable (busy or temporarily blocked)[5]
- 452: Insufficient system storage[5]
- 501: Syntax error in parameters or arguments[5]
- 550: Mailbox is unavailable or not found[5]
- 552: Mailbox is full[5]
- 553: Syntactically incorrect mail address[5]
When debugging batch email sending issues, you can use the cURL code examples provided in the Integration tab of your domain to test and replicate any errors you're experiencing[3].
For a complete implementation, your code should include proper error handling. For example, in PHP implementations, you would typically wrap your API call in a try-catch block to capture and process any exceptions that might occur during the email sending process[2].
Citations:
- 1: https://api-docs.mailtrap.io/docs/mailtrap-api-docs/c323263dc45c5-batch-send-email-including-templates
- 2: https://mailtrap.io/blog/api-send-email/
- 3: https://help.mailtrap.io/article/116-troubleshooting-email-sending
- 4: https://help.mailtrap.io/article/102-webhooks
- 5: https://mailtrap.io/blog/smtp-commands-and-responses/
- 6: https://help.mailtrap.io/article/59-troubleshooting-email-testing
- 7: https://help.mailtrap.io/article/24-mailtrap-testing-api
- 8: https://mailtrap.io/blog/javascript-email-validation/
- 9: https://mailtrap.io/blog/batch-file-send-email/
- 10: https://mailtrap.io/blog/email-headers/
Review error format in batch operation tests
The tests currently mock
errors
asstring[]
, but Mailtrap’s Batch API returns each failed email error as an object containing a numericcode
and a descriptivemessage
.• File:
src/__tests__/lib/mailtrap-client.test.ts
– Around lines 622–630 (and similarly at 651–655), update the mock response:
- Replaceerrors: ["'from' is required", "'subject' is required"]
with an array of{ code: number; message: string }
objects.
- Adjust your assertions to verifyerror.code
anderror.message
properties.
• Confirm the exact field names and types against the Mailtrap Batch Send Email API docs:
https://api-docs.mailtrap.io/docs/mailtrap-api-docs/c323263dc45c5-batch-send-email-including-templates
] | ||
}) | ||
.then(console.log) | ||
.catch(console.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.
.catch(console.error); | |
.catch(console.error); | |
{ | ||
success: true, | ||
message_ids: ["57f17fe0-3935-11f0-0000-f15507b34198"], | ||
}, |
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.
In this case, the expected response would contain two responses
as there are 2 requests
in the request
{ | ||
success: true, | ||
message_ids: ["57f17fe0-3935-11f0-0000-f15507b34198"], | ||
}, |
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.
In this case, the expected response would contain two responses
as there are 2 requests
in the request
responses: [ | ||
{ | ||
success: true, | ||
message_ids: ["57f17fe0-3935-11f0-0000-f15507b34198"], |
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.
In this case, the expected response would contain two responses
as there are 2 requests
in the request
from: { email: "[email protected]", name: "Mailtrap" }, | ||
subject: "Batch Subject", |
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'd remove these 2 lines for the expected response errors to make sense (match real scenario)
from: { email: "[email protected]", name: "Mailtrap" }, | |
subject: "Batch Subject", |
src/lib/MailtrapClient.ts
Outdated
const preparedRequests = request.requests.map((req) => ({ | ||
to: req.to, | ||
cc: req.cc, | ||
bcc: req.bcc, | ||
custom_variables: req.custom_variables, | ||
})); |
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.
What is this mapping needed for? Why are we dropping the rest of the fields that are allowed? Again, base
can contain all the attributes (except recipients) that a regular send
method has and has the same validation rules. requests
can overwrite all of the attributes from base
(except sender).
…g request parameters
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: 0
♻️ Duplicate comments (5)
src/__tests__/lib/mailtrap-client.test.ts (5)
441-449
: Update sandbox mode response schema to match API specification.Similar to the bulk mode test, this response schema is incorrect and needs to be updated to match the actual Mailtrap Batch API response format.
501-509
: Update template batch response schema to match API specification.The response schema used here is also incorrect and should follow the same format as the actual Mailtrap Batch API.
562-574
: Update transactional batch response schema to match API specification.This test also uses the incorrect response schema format and needs to be aligned with the actual API.
376-388
:⚠️ Potential issueUpdate API response schema to match Mailtrap Batch API specification.
The current test uses an incorrect response schema with
success
andresponses
array structure. According to the Mailtrap Batch API documentation, the actual response has a top-levelresponse
object containing batch details and arequestID
string.Apply this diff to correct the response schema:
- const expectedBatchResponseData = { - success: true, - responses: [ - { - success: true, - message_ids: ["57f17fe0-3935-11f0-0000-f15507b34198"], - }, - { - success: true, - message_ids: ["57f17fe0-3935-11f0-0000-f15507b34199"], - }, - ], - }; + const expectedBatchResponseData = { + response: { + emailBatchID: expect.any(Number), + surveyID: expect.any(Number), + userID: expect.any(Number), + batchName: null, + subject: expect.any(String), + description: expect.any(String), + emailText: expect.any(String), + status: expect.any(Number), + batchType: expect.any(Number), + emailCount: 2, + emailFrom: expect.any(String), + creationDate: expect.any(String), + emailStatsList: null, + emailFromName: expect.any(String), + emailBatchStatistics: expect.any(Object), + }, + requestID: expect.any(String), + };
626-634
:⚠️ Potential issueCorrect the error response schema for batch sending.
The error handling test uses an incorrect response structure. Based on the
handleSendingError
function analysis from past reviews, the API should return error responses with anerrors
object containing error arrays, not asuccess: true
wrapper with individual response errors.Apply this diff to correct the error response structure:
- const responseData = { - success: true, - responses: [ - { - success: false, - errors: ["'from' is required", "'subject' is required"], - }, - ], - }; + const responseData = { + errors: { + name: ["'from' is required", "'subject' is required"], + }, + }; + mock.onPost(endpoint).reply(400, responseData);Also update the test expectations:
- // @ts-ignore - const result = await batchClient.batchSend(batchData); - expect(result).toEqual(responseData); - expect(result.responses[0].success).toBe(false); - expect(result.responses[0].errors).toContain("'from' is required"); - expect(result.responses[0].errors).toContain("'subject' is required"); + try { + await batchClient.batchSend(batchData); + } catch (error) { + expect(error).toBeInstanceOf(MailtrapError); + if (error instanceof MailtrapError) { + expect(error.message).toEqual("'from' is required, 'subject' is required"); + } + }
🧹 Nitpick comments (1)
src/__tests__/lib/mailtrap-client.test.ts (1)
653-654
: Remove the @ts-ignore directive.The
@ts-ignore
directive should be removed after correcting the error response handling logic in the previous comment.- // @ts-ignore - const result = await batchClient.batchSend(batchData);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/__tests__/lib/mailtrap-client.test.ts
(1 hunks)src/lib/MailtrapClient.ts
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/MailtrapClient.ts
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/__tests__/lib/mailtrap-client.test.ts (1)
src/lib/MailtrapError.ts (1)
MailtrapError
(1-1)
🔇 Additional comments (1)
src/__tests__/lib/mailtrap-client.test.ts (1)
331-660
: Comprehensive test coverage implemented well.The batch sending test suite provides excellent coverage of different scenarios including mode incompatibility, various sending modes (bulk, sandbox, transactional), template usage, and error handling. The test structure follows good patterns and the batch data structures are well-formed.
src/lib/MailtrapClient.ts
Outdated
base: preparedBase, | ||
requests, |
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.
There can be text
, html
or attachments
on both base
AND single element from requests
but for some reason we only encodeMailBuffers
on base
, why?
Motivation
Integrate Batch Sending API into current node.js SDK to support sending multiple emails in a single request, improving efficiency and reducing API calls.
Changes
batchSend
method toMailtrapClient
classHow to test
npm test
Summary by CodeRabbit
New Features
Documentation
Examples
Tests