Skip to content
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

Email sent but callback is failed #9

Open
endroca opened this issue Jan 31, 2020 · 15 comments
Open

Email sent but callback is failed #9

endroca opened this issue Jan 31, 2020 · 15 comments

Comments

@endroca
Copy link

endroca commented Jan 31, 2020

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
Copy link
Contributor

isocroft commented Feb 4, 2020

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
Copy link
Author

endroca commented Feb 4, 2020

Thank you, I updated the post

@isocroft
Copy link
Contributor

isocroft commented Feb 4, 2020

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
Copy link
Author

endroca commented Feb 5, 2020

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
Copy link
Owner

@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
Copy link
Author

endroca commented Feb 13, 2020

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
Copy link
Owner

stitchng commented Feb 14, 2020

@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
Copy link
Owner

@endroca have you tried the above ? any luck ?

@mavafaei
Copy link

mavafaei commented Mar 14, 2020

any luck

Hi @stitchng
I try this way and error continues.

@synergixe
Copy link

@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
Copy link

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
Copy link
Contributor

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

@synergixe
Copy link

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

@stitchng
Copy link
Owner

stitchng commented Jun 3, 2020

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

@isocroft
Copy link
Contributor

isocroft commented Jan 2, 2021

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants