Why afterSave trigger is run asynchronously of the RestWrite.execute flow?

[cipiripper]

Issue Description

We are migrating app from api.parse.com. In cloud code we have afterSave triggers on all classes, where we set ACL, and other stuff.

When we save a Class object, and when save() is done, we direct user to URL of that object in form of http://server/OBJECT_ID. On parse.com - this works. When object.save() is completed, triggers that set ACL are completed as well, and user can open that URL.

On parse-server, this is not the case. When user is navigated to URL, he gets an error. And if we set timeout like 2sec before navigating user to newly created object, and after save completes, URL works!

We cannot use beforeSave because we need ID of the object being created.

Steps to reproduce

1. Create afterSave trigger on a Class that sets public read ACL on an object in cloud code
2. Create an instance using Parse API, with restricted access ACL
3. Save instance, and when it saves, try to fetch it

Expected Results

User should see his object, because afterSave should set public read ACL when save() is called by user.

Actual Outcome

User gets an error, because ACL is restricted.
If user tries again in 2 sec, he will see this object, because by now afterSave has completed.

Problematic Code (workaround)

So THIS LINE is the root of my problems... I had to return the promise maybeRunTrigger returns, this chained afterSave promise, and made it sync with RestWrite.execute. I also had to accept response in afterSave trigger and call response.success and response.error to resolve a promise...

But I guess this is not a parse thing to do because declaration of afterSave trigger has to change... :)

[flovilmart]

This problem is due to to an architectural change. Initially parse.com was written in GO with a v8 engine spawn at each request. The go web app would likely wait for the execution to terminate and spin out the response at that moment.

Here we're only in JS, so there is no way to wait for the execution of a function to be completed the same way without introducing callbacks (like you did).

We have 2 obvious options here:

1. introduce the response callback in afterSave, afterDelete
2. run the cloudCode functions/hooks inside a separate process/vm, for each hook.

Not sure what the best approach is here. @drew-gross, @nlutsenko what do you think?

[drew-gross]

Probably we need to wait for the after save before returning the response to the client, if that's what api.parse.com does. It's possible that that's not what it does, though, and it just happened to be fast enough, while parse-server isn't. Doing the ACL modification in beforeSave should work as well though.

[flovilmart]

Problem is that we don't expose callbacks in afterSave. I suspect in parse.com the afterSave was run and waiting for the vm process to terminate no?

[drew-gross]

True. Not sure what we can do about that. We could start exposing "done" callbacks, and only require that they be called if they are used or if the afterSave opts in some other way. Jest/Jasmine/Mocha/etc. pull this off by using the length property of the function. A bit janky, but it would work. Or we could just tell everyone to move their afterSave logic that needs to be completed before the next fetch to their beforeSave.

[flovilmart]

done callback seem OK, check if it returns a promise too can work too. It's possible some users have a bad function signature (function(req, res) {}) and would never call done therefore blocking the whole thing.

The Promise return seems to be the safest at the moment.

[flovilmart]

@cipiripper as you started the workaround can you open a PR with the proposed fix?

which is:

if the afterSave/afterDelete function returns a promise, 
wait for the promise to return the response, the promise can resolve or reject, 
that should not affect the outcome of the save operation (it should succeed both ways)

[cipiripper]

@flovilmart Sure.

[cipiripper]

PR: #2499

[flovilmart]

Closed as resolved by #2499