Ntshembo Hlongwane
Ntshembo Hlongwane

Reputation: 1051

NodeJS hangs on second request and does not send response

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

Answers (1)

Kelvin Schoofs
Kelvin Schoofs

Reputation: 8718

While I don't know exactly what causes your error, there's a few things that caught my attention:

  • Every 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.
  • You define 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

Related Questions