Cloud Code import can cause race condition

[Moumouls]

New Issue Checklist

• I am not disclosing a vulnerability.
• I am not just asking a question.
• I have searched through existing issues.
• I can reproduce the issue with the latest version of Parse Server.

Issue Description

Module import is async, importing as module can currently cause a race issue. A developer do not have any guarantee that the code will be loaded before he try to run some cloud code functions. We have some recurrent CI fails due to #7560.

See #7560 (comment)

Steps to reproduce

Actual Outcome

Random CI fails

Expected Outcome

No flacky

Environment

beta

Server

• Parse Server version: 5.0.0-beta.1
• Operating system: all
• Local or remote host (AWS, Azure, Google Cloud, Heroku, Digital Ocean, etc): action

Database

• System (MongoDB or Postgres): all
• Database version: all
• Local or remote host (MongoDB Atlas, mLab, AWS, Azure, Google Cloud, etc): all

Client

• SDK (iOS, Android, JavaScript, PHP, Unity, etc): beta
• SDK version: beta

Logs

[parse-github-assistant]

Thanks for opening this issue!

• 🚀 You can help us to fix this issue faster by opening a pull request with a failing test. See our Contribution Guide for how to make a pull request, or read our New Contributor's Guide if this is your first time contributing.

[Moumouls]

Here as discussed with @mtrezza i'll recommend a simple revert.

[mtrezza]

Thanks @Moumouls. Just in case we want to remove the feature, we don't do pure git reverts, we require a new PR that removes the code. I suggest let's see what @dblythy suggests. I renamed the title because the issue it not purely a CI issue but has potential impact on cloud code loading as I understand it.

[Moumouls]

Yes this could have a real impact in prod environment and could lead to a cloud code crash.

[mtrezza]

The alternative to removing the feature would probably be to get @sadortun's #7525 ready to fix the async initialization of Parse Server. But maybe we'd discover more things to change and would be getting from one thing to the other. So removing the feature seems easiest for now, then re-add it after #7525 has been merged.

[dblythy]

I'm happy for the feature to be reverted (if this is the most suitable approach), however, what's the recommended way for someone who is using Parse Server as a ES6 module to use cloud code? Perhaps we could throw an error "ES6 Modules cannot use cloud code strings, use () => import('') instead.

[Moumouls]

Currently the clean ways is to first import the cloud code file function in a index.js then provide the cloud code function to Parse Server.

ex:

// job.js
export const jobs = () => {
  Parse.Could.Job(xxxxxxxx)
}

// beforeSaves.js
export const beforeSaves = () => {
  Parse.Could.beforeSave(xxxxxxxx)
}

// cloud.js
import { jobs} from './jobs'
import { beforeSaves } from './beforeSaves'

export const cloud = () => {
  // Lazy load all Parse.Cloud
  jobs()
  beforeSaves()
}

// index.js
import { cloud } from './cloud'

// Parse will execute sync the cloud function
// it's a lazy load strategy
ParseServer.start({ cloud })

This is my suggestion @dblythy until we have a clean refactor of ParseServer start 🙂

[sadortun]

I'll have a look at #7525 today. Lmk what's missing to be merged.

[mtrezza]

@sadortun that's great! Mind that we cannot merge any breaking changes in the alpha or beta branch anymore, since we already had the beta release. Any breaking change needs to be phased-in, giving an option to use the current functionality, or the new (breaking) functionality.

However, we need to merge a fix for this issue (#7680) in alpha and beta. So if #7525 is breaking and cannot be phased-in, we would need a solution that does not depend on #7525 to close this issue (#7680), which means probably just reverting.

[sadortun]

@mtrezza FYI, My PR #7525 have no breaking change.

[dblythy]

@sadortun in your PR, you removed the ES6 import() which is a breaking change, as those who use ES6 modules won't be able to specify a cloud code string

[sadortun]

@sadortun in your PR, you removed the ES6 import() which is a breaking change, as those who use ES6 modules won't be able to specify a cloud code string

Noted. That was not intended.

[Moumouls]

@mtrezza @dblythy here i'm sad to announce that ParseServer is currently not ready to support an async cloud code loading correctly without being sure not to break anything on certain productions.

We need an additional PR with a clean refactoring of ParseServer startup. But changing parse server startup from sync to async will lead to a breaking change.

Since we are at beta stage, @mtrezza I think it is wiser to remove the code and open a new issue tracking the refactor needs and how this breaking could be handled without creating any surprise on some production applications.

Here i'll suggest to remove the import code that allow cloud code file as ES Module. 🙁

[mtrezza]

@dblythy If you agree, would you want to open a PR to temporarily remove the feature in the alpha branch? We can re-add this in 2022 when we have more time to prepare #7525 without rush, which I think would be better as it seems to be somewhat tricky.

[mtrezza]

Preparing to revert with #7689 in alpha, #7691 in beta.

[mtrezza]

Closing as #7560 has been reverted in alpha and beta.

Thanks @dblythy for adding the feature, and I hope we can re-add it after #7525 has been merged.