stitchng / adonis-queue

An addon/plugin package to provide driver-based job queueing services in AdonisJS 4.0+
MIT License
33 stars 9 forks source link

Email sent but callback is failed #9

Open endroca opened 4 years ago

endroca commented 4 years ago

Package version

NPM 6.12.1

Node.js and npm version

v10.16.0

Adonis version

4.1

Sample Code (to reproduce the issue)

JOB

/** @type {typeof import('adonisjs-queue/src/Job')} */
const Job = use('Job');

/** @type {typeof import('@adonisjs/mail/src/Mail')} */
const Mail = use('Mail');

class SendEmail extends Job {
  get queue() {
    return 'low';
  }

  constructor(emailAddress, emailFrom, emailSubject, emailTemplate, emailBody) {
    super(arguments);

    this.timeOut = 50; // seconds
    this.retryCount = 1;
    this.retryUntil = 200; // seconds
    // this.delayUntil = Date.parse('2038-01-19T03:14:08.000Z'); // optional, omit this line if not required
  }

  async handle(link, done) {
    console.log(
      `Job [${this.constructor.name}] - handler called: status=running; id=${this.id} `
    );

    link.reportProgress(1);

    let data = link.data; // arguments passed into the constructor
    let error = null;
    let result = null;

    try {
      result = await Mail.send(data.emailTemplate, data.emailBody, message => {
        message.to(data.emailAddress);
        message.from(data.emailFrom);
        message.subject(data.emailSubject);
      });
      link.reportProgress(45);
    } catch (err) {
      link.reportProgress(65);
      throw err;
    } finally {
      link.reportProgress(100);
    }

    return result;
  }

  failed(link, error) {
    console.log(
      `Job [${this.constructor.name}] - status:failed; id=${this.id} `,
      error
    );

    this.detach(); // remove the job from the queue (when the job fails after 3 retries)
  }

  retrying(link, error) {
    console.log(
      `Job [${this.constructor.name}] - status:retrying; id=${this.id} `,
      error
    );
  }

  succeeded(link) {
    console.log(
      `Job [${this.constructor.name}] - status:succeeded; id=${this.id} `
    );
  }
}

module.exports = SendEmail;

Event

/** @type {typeof import('@adonisjs/framework/src/Event')} */
const Event = use('Event');

/** @type {typeof import('adonisjs-queue/src/Queue')} */
const Queue = use('Queue');

const SendEmail = use('App/Jobs/SendEmail');

Event.on('user_registered', async (_email, _body) => {
  await Queue.dispatch(
    new SendEmail(
      _email,
      'support@....com.br',
      'Registro',
      'emails.UserStore',
      _body
    )
  );
});

Response

@@adonisjs/Queue: [Redis] Queue [low] now ready
Job [SendEmail] - handler called: status=running; id=1 
Job [SendEmail] - status:retrying; id=1  undefined
Job [SendEmail] - handler called: status=running; id=1 
Job [SendEmail] - status:failed; id=1  undefined

The email is sent successfully but the callback that is made is invalid

isocroft commented 4 years ago

Please @endroca share the actual code of SendEmail job in your code-base. This will give better context on what is going wrong. Thanks

endroca commented 4 years ago

Thank you, I updated the post

isocroft commented 4 years ago

Currently looked into it. I feel this might be because you are not calling the done(null, result) function inside the handle() method. I will inform @stitchng to update the README on this.

So, for now please update the handle() function of your SendEmail job to look like the below:

async handle(link, done) {
console.log(Job [${this.constructor.name}] - handler called: status=running; id=${this.id});

link.reportProgress(10);

let data = link.data; // arguments passed into the constructor
let error = null;
let result = null;

try {
  result = await Mail.send(data.emailTemplate, data.emailBody, message => {
    message.to(data.emailAddress);
    message.from(data.emailFrom);
    message.subject(data.emailSubject);
  });
  link.reportProgress(50);
} catch (err) {
  link.reportProgress(50);
  error = err;
  result = undefined
} finally {
  link.reportProgress(100);
}

return done(error, result);
}

And then, try again

Let me know the outcome of this please @endroca . Thank you

endroca commented 4 years ago

The error continues =(

info: serving app on http://127.0.0.1:3333
@@adonisjs/Queue: [Redis] Queue [low] now ready
Job [SendEmail] - handler called: status=running; id=1
Job [SendEmail] - status:retrying; id=1  undefined
Job [SendEmail] - handler called: status=running; id=1
Job [SendEmail] - status:failed; id=1  undefined
info: @@adonis/Queue: removing job from queue...

I used the same function that you described

  async handle(link, done) {
    console.log(
      `Job [${this.constructor.name}] - handler called: status=running; id=${this.id}`
    );

    link.reportProgress(10);

    const { data } = link; // arguments passed into the constructor
    let error = null;
    let result = null;

    try {
      result = await Mail.send(data.emailTemplate, data.emailBody, message => {
        message.to(data.emailAddress);
        message.from(data.emailFrom);
        message.subject(data.emailSubject);
      });
      link.reportProgress(50);
    } catch (err) {
      link.reportProgress(50);
      error = err;
      result = undefined;
    } finally {
      link.reportProgress(100);
    }

    return done(error, result);
  }

The funny thing is that I get two emails, for the failure attempt

stitchng commented 4 years ago

@endroca Please can you update the handle function as below:


async handle(link, done) {
    console.log(
      `Job [${this.constructor.name}] - handler called: status=running; id=${this.id}`
    );

    link.reportProgress(10);

    const { data } = link; // arguments passed into the constructor
    let error = null;
    let result = null;

    try {
      result = await Mail.send(data.emailTemplate, data.emailBody, message => {
        message.to(data.emailAddress);
        message.from(data.emailFrom);
        message.subject(data.emailSubject);
      });
      link.reportProgress(50);
    } catch (err) {
      link.reportProgress(50);
      error = err;
      result = undefined;
    } finally {
      link.reportProgress(100);
      done(error, result);
    }
  }

It should be fine now

endroca commented 4 years ago

I understand that with asynchronous call sending email it really makes sense to place the return function within the finally

    } finally {
      console.log('success');
      link.reportProgress(100);
      done(error, result);
    }
@@adonisjs/Queue: [Redis] Queue [low] now ready
Job [SendEmail] - handler called: status=running; id=1 
Job [SendEmail] - status:retrying; id=1  undefined
Job [SendEmail] - handler called: status=running; id=1 
Job [SendEmail] - status:failed; id=1  undefined
info: @@adonis/Queue: removing job from queue...
success
success

However, the system returns the error function even before calling the "done" function

stitchng commented 4 years ago

@endroca confirming from the bee-queue repo. The handle function needs to return a Promise. Also from the reportProgress() function returns a promise as well and needs to be "awaited".

See below:

async handle(link) {
    console.log(
      `Job [${this.constructor.name}] - handler called: status=running; id=${this.id}`
    );

    await link.reportProgress(10);

    const { data } = link; // arguments passed into the constructor
    let error = null;
    let result = null;

    try {
      result = await Mail.send(data.emailTemplate, data.emailBody, message => {
        message.to(data.emailAddress);
        message.from(data.emailFrom);
        message.subject(data.emailSubject);
      });
      await link.reportProgress(50);
    } catch (err) {
      error = err;
      result = undefined;
      await link.reportProgress(50);
    } finally {
      await link.reportProgress(100);
      return error === null ? Promise.resolve(result) : Promise.reject(error);
    }
  }
stitchng commented 4 years ago

@endroca have you tried the above ? any luck ?

mavafaei commented 4 years ago

any luck

Hi @stitchng I try this way and error continues.

synergixe commented 4 years ago

@stitchng @isocroft i think you need to update bee-queue dependency to the latest version and release a new version. The errors seem to be coming from the bee-queue library.

afolabiabass commented 4 years ago

Any solution here, having the same problem. For now I have stopped it sending the email more than once by setting retryCount to '0' if you set to 0 default value of 2 replaces it (This in itself is an issue)

isocroft commented 4 years ago

A new version of the adonis-queue library has just been released - v0.1.10 on NPM. @endroca @afolabiabass @mavafaei @stitchng

synergixe commented 4 years ago

@endroca @afolabiabass can you give feedback on the latest release please ? Do the errors still persist ?

stitchng commented 4 years ago

@mavafaei @afolabiabass @endroca how is the new version for adonis-queue (v0.1.10) working ? Still having errors ?

isocroft commented 3 years ago

Hello @endroca @mavafaei . Happy New Year to you all. Are you still experiencing these issues ? Please provide more info thanks