wildhart / meteor.jobs

A simple job scheduler for Meteor.js
MIT License
18 stars 9 forks source link

Job State Transitions #10

Closed 67726e closed 3 years ago

67726e commented 3 years ago

In reading the documentation, I'm not sure what exactly I'm supposed to be doing in regards to jobs. From testing, it appears that the default behavior requires one to explicitly set the state as a part of the job. For example, if I run:

Jobs.register({
    performAction: function() {
      console.log('Working!');
    },
});

Jobs.run('performAction');

I will receive a warning in the logs:

W20210326-08:19:50.033(-4)? (STDERR) Jobs Job was not resolved with success, failure, reschedule or remove

Which finishes with my job in an failure state. This is unclear, so it perhaps is worth documenting at a minimum. Perhaps this is a difference between conventions with other Node job runners and job runners in other languages (Java, C#, Python, Ruby) but my experience with job runners is that they would handle state transition on your behalf.

// This should end with my job in a `success` state
performAction() { /**/ }
// This should end with my job in a `success` state
performAsyncAction: async function() {
  return await new Promise((resolve) => Meteor.setTimeout(() => resolve, 5000));
}
// This should end with my job in a `success` state
performPromiseAction: function() {
  return new Promise((resolve) => Meteor.setTimeout(() => resolve, 5000));
}

// This should end with my job in a `failure` state
performOtherAction() { throw Meteor.Error('...', '...') }
wildhart commented 3 years ago

In the Jobs.Register section of the docs it does say:

Each job must be resolved with success, failure, reschedule, and/or remove.

So yes, you are required to explicitly set the state of a completed job. If you want to keep the job in the db call this.success(), otherwise call this.remove(). This is the behaviour I inherited from msavin:sjobs to which I purposefully kept the API as similar as possible.

I might consider adding a defaultCompleteState configuration item which could take either 'success' or 'remove'.

67726e commented 3 years ago

Thank you for the consideration with this item. I'm currently using a function that handles things on the user-side.

const withCompletion = (method) => function() {
        try {
                const result = method.apply(this, arguments);

                if (result && result.then && result.catch) {
                        // Promise-like result, find the result to complete
                        result.then((value) => {
                                this.success(result);
                        }).catch((value) => {
                                this.failure(result);
                        });
                } else {
                        // Non-Promise result, success
                        this.success(result);
                }
        } catch (exception) {
                // Exception, failure
                this.failure(exception);
        }
};

Then:

Jobs.register({
  // import { compose } from 'ramda';
  // import performAction from './performAction';
  performAction: compose(
    withCompletion,
  )(performAction),

  // Alternatively, just manually compose
  performAction: withCompletion(performAction),
});
wildhart commented 3 years ago

I've added the defaultCompletion: 'success' | 'remove' option and released version 1.0.11 to Atmosphere.

Note that jobs which throw an error are automatically marked as 'failed'.

67726e commented 3 years ago

Thank you for putting together the feature! Unfortunately, I'm running into some trouble installing it with the latest Meteor:

user@VirtualBox:~/Documents/application/meteor-two$ meteor add wildhart:jobs@1.0.11
 => Errors while adding packages:

While selecting package versions:
error: Conflict: Constraint typescript@3.7.6 is not satisfied by typescript 4.1.2.
Constraints on package "typescript":
* typescript@4.1.2 <- top level
* typescript@~4.1.2 <- top level
* typescript@3.7.0 || 4.1.2 || 4.1.2 <- react-meteor-data 2.2.2
* typescript@3.7.6 <- wildhart:jobs 1.0.11

Haven't dug into the specifics, but it looks like one can use 3.7.x series or 4.1.x series Typescript as dependencies based on the react-meteor-data package?

wildhart commented 3 years ago

I've published 1.0.12 which should be able to use typescript 3 or 4, so try that...

67726e commented 3 years ago
Changes to your project's package version selections from updating package versions:

typescript     added, version 4.1.2
wildhart:jobs  upgraded from 1.0.9 to 1.0.12

Success! Thank you again!