Modernization of hooks and functions

[flovilmart]

Is your feature request related to a problem? Please describe.

Right now, we still rely on the legacy backbone style success / error callback methods. With node.js moving forward at an incredible pace, I believe it's time to make the cloud functions enter the 21st century supporting async / await, promises and more

Describe the solution you'd like

• Support promises, async/await in cloud code as valid return values for hooks and functions
• Mark calls to success / error callbacks deprecated in Cloud Code (with a big fat warning)
• Later on, remove success / error implementations

Cloud funcitons would look like:

Parse.Cloud.beforeSave('ClassName', async ({ object }) => {
    await doSomething();
    if (somethingsFishy(object)) {
        throw new Error('OOps')
    }
    return;
});

// Also valid
Parse.Cloud.beforeSave('ClassName', async ({ object }) => {
    await doSomething();
    if (somethingsFishy(object)) {
        return new Error('OOps'); // returning an error is also allowed
    }
});

For functions:

Parse.Cloud.define('MyFunction', async ({ params }) => {
    await doSomething();
    if (!param.isValid) {
        return new Error('OOps'); // returning an error is also allowed
    }
    return 'OK!';
});

cc: @acinader , @dplewis

[SebC99]

That would be great!
In my own code, as I wanted to use promises, I've defined this helper methods:

// Create a new parse cloud method with a promise
function createParseCloudFunction(name, promise) {
  return Parse.Cloud.define(name, (request, response) => {
    return promise(request)._thenRunCallbacks(sendResponse(response));
  });
}

// Create a generic response from a promise
function sendResponse(response) {
  return {
    success: function(result) {
      response.success(result);
    },
    error: function (error) {
      response.error(error);
    }
  }
}

which allows me to add a new cloud function by calling

createParseCloudFunction("MyFunction", {params, user, master} => { ... do something ... });

And I also have one for jobs:

function createParseJob(name, promise) {
  return Parse.Cloud.job(name, function(request, status) {
    return promise(request)._thenRunCallbacks(sendResponse(status));
  });
}

[flovilmart]

@SebC99 with the next release of the server the _ThenRunCallbacks will fail

[SebC99]

@flovilmart oh? You're removing Parse.Promise? Is there somewhere where this is described? Thanks for telling me anyway!! 🙏
EDIT: I've just saw #4925 well done!!

[oallouch]

Everybody's got a callback-to-promise wrapper.
No big deal, but if it's made core, it's a plus.

[flovilmart]

The thing is with the next version of the JS SDK, callbacks won’t work at all, the underlying code is removed.

I’ll update the README as well in the JS SDK

[flovilmart]

So there's this opened PR now #4933

Anyone wanting to do the migration and see how it goes?

You can use the https://github.com/parse-community/parse-server#version-3.0.0 branch in your package.json

[srameshr]

Promises, async/await in cloud code are already working.
Here is a chunk of code / pattern that I have been using for a long time:

Parse.Cloud.define('put_content', async (req, res) => {
      try {
        const data = await this.put_content(req, res);
        res.success({ data });
      } catch (e) {
        const { code, message } = errors.constructErrorObject(e.code || 500, e);
        errors.handleError({ code, message }, res);
      }
    });
  }

[flovilmart]

@srameshr yes, obviously, if you are using node > 8 you have async await. But the goal here is to remove .success and .error as they are from a time backbone style callbacks were all the rage. as we have promised and async await in node we don’t need this style.

[SebC99]

@flovilmart are you removing the Parse.Promise as well to replace them with native promise?
Then we'll have to change all the .done callback to .then callback, right?

[flovilmart]

Yes indeed, this comes with the JS SDK 2.0

[cleever]

Great! I have been writing some parse server code with typescript ( using async/await even if node < 8) and it will be awesome throw away some Parse.Promise wrappers and all that backbone style code.

[flovilmart]

I'll leave the branch open this week, before we make a release, this ways if there are issues or else, we should be able to fix them before hand.

[flovilmart]

You can all check the migration docs: https://github.com/parse-community/parse-server/blob/3e5d2581553ed41185b9cf60c4bd9ceb12f8152e/3.0.0.md

[SebC99]

@flovilmart I'll try to test it in our environment, but I have to migrate all Parse.Promise first
It may be useful to add a warning about it in the migration docs too

[flovilmart]

It will completely explode in any case if you’re using backbone style callbacks or ParsePromises

[SebC99]

That's why I suggested a warning: the migration tells to update the Parse.cloud methods as there is no more backbone style callbacks, but the other reason is the Parse.Promise some people might have elsewhere in their cloud code

[flovilmart]

So we should make a migration guide on the JS SDK instead and add a link there

[flovilmart]

@SebC99 what do you think of this: https://github.com/parse-community/Parse-SDK-JS/blob/13a250069129e5adf2f02571b5ccb767f33845cb/2.0.0.md

[SebC99]

Awesome!
The only improvement I see is to precise that Promise.all takes only array (iterable) whereas Parse.Promise.When could work with multiple arguments. But it may be a very small issue.

[flovilmart]

I’ve added it

[SebC99]

Perfect for me!

[flovilmart]

Awesome, @SebC99 let me know how the migration goes and if you encounter any issue.

[oallouch]

Perfect for me too.

[flovilmart]

@oallouch did you run the transition on your codebase? Any issue?

[oallouch]

Nope, and I already had a small wrapper to handle logging, so it was fast.

[SebC99]

@flovilmart sorry for the delay, I had a lot of updates to do with the promises (that was the hardest part as I had a lot of them using arguments lists instead of arrays).

I haven't test everything, but 90% of it (I still have jobs to test) and for now it works perfectly!

[flovilmart]

@SebC99 did you know this trick with the array spread?

Promise.all([query1, query2, query3]).then(([result1, result2, result3]) => {
});

Glad to hear everything is 👍 for you!

[SebC99]

Yes that's what I used everywhere! But I had to add the [] everywhere ;)
This is even better:

[result1, result2, result3] = await Promise.all([query1, query2, query3]);

👌

[SebC99]

@flovilmart there's an issue (well something to warn about) with Parse.Promise.always vs Promise.finally -> always would have parameters when calling the callback, whereas finally have no parameters. So it's not exactly the same.

And I have a tricky bug here:

const obj1 = new Parse.Object("class1");
const obj2 = new Parse.Object("class2");
obj2.set("obj1", obj1);

return obj1.doSomething().finally(() => obj2.save());

doSomething() is a method that will return an updated obj1.

With Parse.Promise.always, the result would be a promise with the obj2 as a result, containing a pointer to obj1.

With Promise.finally, the result is a promise with the obj1 as a result, instead of the obj2!!!

[flovilmart]

Ok, can you open a PR on the JS repo for the migration guide? https://github.com/parse-community/Parse-SDK-JS/blob/master/2.0.0.md

[SebC99]

I'll try do it this weekend. As for my tricky thing, after reading lots of documentation, it appears finally will not change the result of a promises chain, so a trailing then would have the previous result as a parameter instead of the finally result.

[danepowell]

I have also been using async cloud functions as suggested by @srameshr for some time now with Node.js 8. However, I recently tried to do the same with a cloud job and it's not behaving at all as expected. Basically, queries within the job don't seem to be obeying await commands. I'm wondering if this issue will fix that, or if something else is going on? I'd love any suggestions: https://stackoverflow.com/questions/51832572/trouble-with-async-and-parse

Sorry for the mild hijack.

[flovilmart]

In your case it’s lambda killing your process. Because, well, job return instantly, and execute in the background. As there’s no ‘background’ in lambda, your job gets killed. Nothing to do with async.

[danepowell]

That doesn't really make sense to me, since I've defined the job as async and all of my calls to other async functions within the job use await. So the job shouldn't be returning instantly, assuming Parse Server is properly calling the job using await or Promises.

Additionally, I've confirmed that the exact same code works fine as a cloud function but not a cloud job, which leads me to think this is a bug. I've opened #4995

[flovilmart]

I've explained why it isn't a bug it's by design and on purpose. So you can run things longer that the httpRequest timeout.

If you have a look at the code here you'll see that the handleCloudJob() function goes to great length to isolate the job execution from the request life cycle. This is BY DESIGN and NOT A BUG.

Now, let's see how it works in lambda:

1. Request comes in
2. handleCloudJob() is called
3. The job is set a running
4. We call process.nextTick() to enqueue the job effective work
5. we return the job status ID.
6. the response is sent with the jobID
7. the job work starts, enqueued by process.nextTick
   ... > time passes
8. the job finishes
9. the jobStatus gets updated

Now imagine for a minute we didn't do that:

at if instead of 6. Sending the response, we were 'awaiting' on the job, like you suggest

What would happen is:

• The timeout for the socket would probably be reached (30s), as jobs are 'long'
• the connection would be closed
• impossible to get the job ID.

Now what happens in you lambda environment:

• the enqueuing is done at step 4,
• at step 6 the response is sent
• the response is sent to the client
• the lambda environment is destroyed.

Which is the expected behaviour.

Can it be better? Perhaps, but that requires a totally different execution environment on jobs. basically worker that await for messages that functions that executes for long time in the background (so not front-end lambdas)

In the lambdas FAQ's it's stated that the default timeout is 3s and can be extended to max 300s. So by default lambda isn't suited to run jobs.

While I understand your frustration and that it doens't make sense to 'you', it's all by design.

Even if we swapped the implementation on parse-server it wouldn't make it suited for lambdas to run jobs.