SimonErm / react-native-job-queue

Easy to use react-native queuing library
https://simonerm.github.io/react-native-job-queue/
122 stars 33 forks source link

All jobs go to the onSuccess event, never to the onFailure event, even if the function that is executed in the payload returns a reject() #62

Closed ArkaitzSKR closed 2 years ago

ArkaitzSKR commented 2 years ago

All jobs go to the onSuccess event, never to the onFailure event, even if the function that is executed in the payload returns a reject()

var jobId = QueueClassInstnace.addJob(QueueClassInstnace.workername, jobs().sendReadToAPi.work(), { attempts: this.attempts, timeout: this.timeout, priority: this.priority }, true)

Work function :

  async work(): Promise<void> {
        return new Promise(async (resolve, reject) => {
          var response = await instance().user.getUserInfo()
             if (response){
               return resolve()
           }else{
             return reject()
        }
}

Can you give an example of how to send a job to onFailure whose result is not what you want?

Thanks!

SimonErm commented 2 years ago

Can you try if it works if you throw an error instead of rejeecting the promise?

ArkaitzSKR commented 2 years ago

Yes, I have already tried it and nothing. throw new Error(Job error); The truth is that there is something strange. When I debug, the response output from the function and what the worker receives are not the same. Making the response a CancellanlePromise misses something and then is not able to detect it as reject.

In addition, in the attemps of the failed jobs the function is not executed several times, it only reads the result of the first execution over and over again.

SimonErm commented 2 years ago

I will try to reproduce this

SimonErm commented 2 years ago

Can you provide the code of your queue configuration?

ArkaitzSKR commented 2 years ago
queue.addWorker(
new Worker(workerName, async (payload) => {
            return new Promise<void>((resolve) => {
                setTimeout(() => {
                    console.log(payload.text);
                    resolve();
                }, payload.delay);
            });
        }, 
        {
            onStart: async ({ id }) => {
                console.log('onStart', id, job)
            },
            onSuccess: async ({ id }) => {
                    console.log('onSuccess', id, jobs)
            },
            onFailure: async ({ id }, error) => {
                console.log('onFailure', id, error, jobs)
            },
            onCompletion: () => {
                console.log('onCompletion')

            },
        }
        ))
ArkaitzSKR commented 2 years ago
queue.configure(
            {
                onQueueFinish: (executedJobs: any[]) => {
                    console.log("Queue stopped and executed", executedJobs)
                }
            }
        );
SimonErm commented 2 years ago

I modified https://github.com/SimonErm/react-native-job-queue/blob/3b063a71c8f67c50b9b32e855cd931815c84a931/src/__tests__/Queue.test.ts#L144 to use your configuration, but wasn't able to reproduce the issue. Can you provide a minimal reproduction sample repo?

ArkaitzSKR commented 2 years ago

Very well, I'll do tests and I'll tell you.

When I have been debugging I have found that the input payload was a promise but when doing const executerPromise = this.executer(job.payload); The value became undefined. So it always went to success, it never caught the reject().

 execute(rawJob: RawJob) {
        const { timeout } = rawJob;
        const payload: P = JSON.parse(rawJob.payload);
        const job = { ...rawJob, ...{ payload } };
        this.executionCount++;
        this.onStart(job);
        if (timeout > 0) {
            return this.executeWithTimeout(job, timeout);
        } else {
            return this.executer(payload);
        }
    }
    private executeWithTimeout(job: Job<P>, timeout: number) {
        let cancel;
        const promise: CancellablePromise<any> = new Promise(async (resolve, reject) => {
            const timeoutPromise = new Promise((resolve, reject) => {
                setTimeout(() => {
                    reject(new Error(`Job ${job.id} timed out`));
                }, timeout);
            });
            const executerPromise = this.executer(job.payload); <- this line
            if (executerPromise) {
                cancel = executerPromise[CANCEL];
                try {
                    await Promise.race([timeoutPromise, executerPromise]);
                    resolve(true);
                } catch (error) {
                    // cancel task if has cancel method
                    if (executerPromise[CANCEL] && typeof executerPromise[CANCEL] === 'function') {
                        executerPromise[CANCEL]!();
                    }
                    reject(error);
                }
            }
        });
        promise[CANCEL] = cancel;
        return promise;
    }

But instead if I change the worker so that from the payload it receives a true or an error instead of a promise and in the worker I generate a resolve or reject promise, in that case I can send the jobs to failure.

async execute(rawJob: RawJob) {
    const { timeout } = rawJob;
    const payload: P = JSON.parse(rawJob.payload);
    const job = { ...rawJob, ...{ payload } };
    this.executionCount++;
    this.onStart(job);
    if (timeout > 0) {
      return this.executeWithTimeout(job, timeout);
    } else {
      var rsp =  payload
      if (rsp==true) {
        return new Promise(async (resolve, reject) => { resolve(true) })
      } else {
        return new Promise(async (resolve, reject) => { reject(rsp) })
      }
    }
  }
  private executeWithTimeout(job: Job<P>, timeout: number) {
    let cancel
      const timeoutPromise = new Promise((resolve, reject) => {
        setTimeout(() => {
          return reject(new Error(`Job ${job.id} timed out`));
        }, timeout);
      });
      const executerPromise = job.payload
      if (executerPromise==true) {
        return new Promise(async (resolve, reject) => { resolve(true) })
      } else {
        return new Promise(async (resolve, reject) => { reject(executerPromise) })
      }
  }

This code is done quickly and badly but for what I need it works, when I have time I will rethink it. I have not been able to solve the attemps problem, that instead of reading 3 or as many times as the result of the first execution, I execute it that number of times.

SimonErm commented 2 years ago

Sorry I forgot to answer. Did you find out anything new? Can you provide an example how you create a job?

SimonErm commented 2 years ago

Close this for now due to inactivity.