Reputation: 1051
Hey there can I please get some assistance I am failing to understand why my server is failing to send response when I hit the same route more than 1.
so I have a NodeJS server where by I have an event emitter setup for sending email notifications something like this below(In another fail):
import EventEmitter from 'events';
import { createTransport, Transporter, SendMailOptions } from 'nodemailer';
import { Response } from 'express';
export const emitter = new EventEmitter();
emitter.on('email-notification', (message: SendMailOptions) => {
const transpoter: Transporter = createTransport({
service: 'SendinBlue',
auth: {
user: process.env.sendin_blue_login,
pass: process.env.sendin_blue_pass,
},
});
transpoter.sendMail(message, (error, info) => {
if (error) {
emitter.emit('email-notification-failed', error);
}
emitter.emit('email-notification-success', info);
});
});
Now I have a route PUT: /api/shipment/status/:order_id/:status
where I want update shipment status now on the first request everything run smoothly I get my email notification sent and server also sending response based on the emitter structure given when email is sent successfully or not
But the problem comes when I send request for the second or more I get the email notification sent but now the problem is the server hangs and does not give back response
Below is my controller on how I am handling updating of status
async update_package_status(request: any, response: Response) {
/**
*STATUS -> 1: PENDING
-> 2: CANCELLED
-> 3: COMING
-> 4: ARRIVED
**/
const status =
request.params.status === '1'
? ShipmentStatus.PENDING
: request.params.status === '2'
? ShipmentStatus.CANCELLED
: request.params.status === '3'
? ShipmentStatus.COMING
: ShipmentStatus.ARRIVED;
const order_id = request.params.order_id;
try {
const updatedDoc = await shipmentModel.findOneAndUpdate({ _id: order_id }, { $set: { status: status } }, { new: true });
const user = await userModel.findOne({ _id: updatedDoc.owner });
const html = `
<h1/>Your package status has changed</h1>
<ul style="list-style:none;">
<li/>Current status: ${updatedDoc.status}</li>
<li/>Current location: ${updatedDoc.current_location}</li>
<li />Previous location: ${updatedDoc.travelled_locations[updatedDoc.travelled_locations.length - 2]}
<li style="font-weight:bolder;"/>Shipment Total: ${updatedDoc.price}</li>
</ul>
`;
const message: SendMailOptions = {
from: '[email protected]',
to: user.email,
subject: 'Package status update',
html,
priority: 'high',
replyTo: '[email protected]',
};
emitter.on(Event.EMAIL_NOTIFICATION_FAIL, (error) => {
console.log('FAILED TO SEND EMAIL !!!! @STATUS UPDATE');
return response.status(500).json({ msg: `Network Error: Failed to notify ${user.email} of their account being created` });
});
emitter.on(Event.EMAIL_NOTIFICATION_SUCCESS, (info) => {
console.log('EMAIL SENT @STATUS UPDATE');
return response.status(200).json({ msg: `Email notification sent to ${user.email} for status update` });
});
} catch (error) {
return response.status(500).json(error);
}
}
Upvotes: 0
Views: 107
Reputation: 8718
While I don't know exactly what causes your error, there's a few things that caught my attention:
update_package_status
call, you add two event listeners to
your emitter
. Not only do you never remove them, but these handlers
will also receive success/fail events from other attempts at
sending mails.message
but I don't actually ever see you emitting the
email-notification
event.For the first problem, you could make your sender an asynchronous function:
async function sendEmail(message: SendMailOptions) {
const transporter: Transporter = createTransport({
service: 'SendinBlue',
auth: {
user: process.env.sendin_blue_login,
pass: process.env.sendin_blue_pass,
},
});
return new Promise((resolve, reject) => {
transporter.sendMail(message, (error, info) =>
error ? reject(error) : resolve(info));
});
}
Which you could then use in update_package_status
using try/catch
or .then(info => {}).catch(error => {})
.
Alternatively, if you really want to go with the event-based approach, pass a unique ID during the initial event emit, which you'll then listen for in the success/failure event handlers. Here I'm wrapping the "how to handle the right event" part in an asynchronous function you can adapt for your needs:
function sendEmail(message: SendMailOptions) {
return new Promise((resolve, reject) => {
// Could use an external counter you always increase, but this should be fine
// if you don't send two emails to the same person in the same millisecond.
const uniqueId = `${message.to}-${Date.now()}`;
const makeHandler = (callback: any) => (data: SendMailReceipt | Error, forUniqueId: string) => {
// Replace the SendMailReceipt here ^ with the proper type
if (forUniqueId !== uniqueId) return; // Not in response to our attempt
// Disconnect our event listeners, don't need them anymore
emitter.off(Event.EMAIL_NOTIFICATION_SUCCESS, onSuccess);
emitter.off(Event.EMAIL_NOTIFICATION_FAIL, onFail);
callback(info);
};
const onSuccess = makeHandler(resolve);
const onFail = makeHandler(reject);
// Actually add the listeners
emitter.on(Event.EMAIL_NOTIFICATION_SUCCESS, onSuccess);
emitter.on(Event.EMAIL_NOTIFICATION_FAIL, onFail);
// Actually emit the event to try to attempt the mail now
// (doing it before adding our event listeners should be fine because
// it shouldn't all be synchronous, but just in case)
emitter.emit(Event.EMAIL_NOTIFICATION, message, uniqueId);
});
}
That assumes you also emit the uniqueId
back in your sendMail
callback.
Upvotes: 1