Parse Server may expose HTTP routes before complete initialization

[sadortun]

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

Currently, all ParseServer initialization is done in the constructor and there is no way to ensure everything is ready before the app is loaded.

The main issue(s)

• Parse Server constructor will continue to do initialization tasks long after the constructor caller returns, and we have no way to await for its completion.

• Server will start to listen to incoming http requests before CloudCode is loaded.

---

This can be bad in some case because express and ParseServer can get exposed to the world, before it have completed its initialization leading to client side errors

Ideally, we should refactor the constructor startup process into a async start() method that can be awaited on.

Currently we see at occasion during rolling updates clients running cloud functions Parse.Cloud.run('xyz') and receiving Parse.Error Function XYZ does not exists. This is caused by having the parse api mounted and express running before all Cloud Code have a time to start.

A few solutions

(A) Without introducing a BC Break

This solution is great as a compromise for now.. It's simple, and have no long term implications.

Add a isReady() function that return a promise that is resolved when the constructor has completed

class ParseServer {
  private isReady : Promise<void>
   constructor(){
       this.isReady = new Promise<void>(resolve => {
          // ... ALL CURRENT CONSTRUCTOR CODE
       })       
   }
   
   public function isReady () : Promise<void> {
      return this.isReady;
   }
}

// ......  server.ts
const api = new ParseServer(config)
await api.ready()

(B) Without introducing a BC Break, and a path forward

This solution is great long term solution.

Move constructor init into a start() method, and add a ParseServerOption autoStart: true. Later in version 6.0.0 we can default autoStart to false or ignore/deprecate the parameter

class ParseServer {
   constructor(options){
      if(options.autoStart === true){
          this.start()
      }
   }
   
   public function start() : Promise<void> {
       // ... ALL CURRENT CONSTRUCTOR CODE ...
   }
}

// ......  server.ts
const config = { autoStart : false };
const api = new ParseServer(config)
await api.start()

(C) Pure and simple BC Break

Just put all constructor code into a start() and tell users they need to call it in the 5.0.0 release notes

const api = new ParseServer(config)
await api.start()

---

(C) is obvisously a bit harsh 😆 ! LMK what you think and i can submit a PR for either (A) or (B)

Steps to reproduce

Actual Outcome

Expected Outcome

Failing Test Case / Pull Request

• 🤩 I submitted a PR with a fix and a test case.
• 🧐 I submitted a PR with a failing test case.

Environment

Server

• Parse Server version: 4.5.0
• Operating system: linux/windows
• Local or remote host (AWS, Azure, Google Cloud, Heroku, Digital Ocean, etc): local a d GC

Database

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

Client

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

Logs

Have a good day,
Samuel

[github-actions]

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.

[mtrezza]

Is this actually a bug or an enhancement? In other words, is there currently something not working with Parse Server (bug), or is it not usable for your specific use case (enhancement)?

[sadortun]

Heh, it's a little bit of both!

It's a bug in the sense of having Parse Server constructor will continue to do initialization tasks long after the constructor caller returns, and we have no way to await for its completion.

It's also a bug in the sense that the Server will start to listen to incoming http requests before CloudCode is loaded.

And it's a feature request in the sense that it would be a good thing to improve for the future versions, and would make the initialization behaviour deterministic instead of 'random' ( If the database take a few seconds to connect for example)

[mtrezza]

It's also a bug in the sense that the Server will start to listen to incoming http requests before CloudCode is loaded.

This sounds much like a bug. Thanks for explaining. Could you clarify this in the issue description?

[sadortun]

✔️ updated

[mtrezza]

I classified this as a bug with severity S3 (normal), because:

• The workaround is to add a delay on the infrastructure level before routing requests to Parse Server, to give it enough time to start-up and load its components; or query the server status endpoint to know when Parse Server is ready

[davimacedo]

Have you looked at serverStartComplete option? Something like the following would probably solve your issue so I don't really think it is a bug, but, of course, can be improved.

let parseServer;
let expressServer;

export async function start(): Promise<void> {
  await new Promise<void>((resolve, reject) => {
    try {
      parseServer = new ParseServer({
        serverURL: 'http://localhost:1337/parse',
        appId: 'someappid',
        javascriptKey: 'somejavascriptkey',
        masterKey: 'somemasterkey',
        databaseURI: 'mongodb://localhost:27017/pase',
        cloud: './cloud/main.js',
        serverStartComplete: (error) => {
          if (error) {
            reject(error);
          } else {
            resolve();
          }
        },
      });
    } catch (e) {
      reject(e);
    }
  });

  const app = express();

  app.use('/parse', parseServer.app);

  return new Promise<void>((resolve, reject) => {
    try {
      expressServer = app.listen(1337, resolve);
    } catch (e) {
      reject(e);
    }
  });
}

[sadortun]

Hi @davimacedo,

Thanks for your suggestion! Yes, that would solve part of the issue, but Cloud Codes will still load after the serverStartComplete is triggered.

In general, performing async operations in a class constructor is a bad idea. My above suggestion (B) would address that, and ensure all proper async code can be awaited in a proper way.

---

For my (A) suggestion, would be a minimal impact of you don't intend in having a distinct start() method. At first I thought it was just a workaround, but it seems it's a documented design pattern

LMK what would be your prefered approach and i'll submit a PR this week.

[mtrezza]

Currently we see at occasion during rolling updates clients running cloud functions Parse.Cloud.run('xyz') and receiving Parse.Error Function XYZ does not exists.

Reading again the original issue, this sounds more like an infrastructure issue, because if the express app does not listen to the port yet, you probably wouldn't want to route external requests to the server instance yet. I can think of 3 design patterns that are usual in an infrastructure context:

• a) Parse Server calls an infrastructure API to announce when it is ready to receive requests (push)
• b) Make an infrastructure component poll a Parse Server API to know when it is ready to receive requests (pull)
• c) Wait for a fixed amount of time that is usually takes for the server app to be ready before routing requests to the server; that is for server apps that can neither do (a) nor (b), but luckily that doesn't apply to Parse Server

For (a) I assume that it would be serverStartComplete that can call an API. For (b) I assume it would be the Parse Server /status endpoint that can be polled.

If serverStartComplete is called before everything is loaded or the /status endpoint indicates an incorrect readiness state, that would be the issue to address in my opinion.

However, regardless of that issue, there may be potential for improvement in the Parse Server constructor design, to give more versatility on the node level to control the readiness state, but that would probably be an enhancement that is separate from the aforementioned bug. If fixing the bug doesn't depend on a constructor redesign, I suggest to address the enhancement in a separate GitHub issue.

[sadortun]

Reading again the original issue, this sounds more like an infrastructure issue, because if the express app does not listen to the port yet

This is a ParseServer issue. Express starts to listen to requests before ParseServer finished it's loading.

Currently, it's not that of a big deal because overall, initialization is fairly quick 1-2 seconds.

But in the case of Parse not being able to connect to Mongo, Parse and Express should not listen to requests before the server is ready OR report a healthy status when in reality any requests would fail due to the lack of database connection.

Secondly, this issue/improvement is directly related to the Schema Migration PR. With the introduction of schema migrations, you definitively DONT want Cloud code to load before migrations are completed, and you don't want Express to listen to requests before Cloud code is loaded.

Schema Migrations are quite fast, but if you have 50 Schemas to validate/update it may take a 10-15 seconds.

The fixes/improvements proposed above would fix theses issues.

[mtrezza]

This is a ParseServer issue.

I agree if one of the following is true, like I said above:

If serverStartComplete is called before everything is loaded or the /status endpoint indicates an incorrect readiness state

I think you confirmed that already in your previous comment:

Cloud Codes will still load after the serverStartComplete is triggered.

Does the constructor need to change to fix the callback issue?

[sadortun]

@mtrezza

Does the constructor need to change to fix the callback issue?

Good question, IMO serverStartCpmplete should be called once everything is actually ready. Currently, it's not the case since it's called before ParseCloud.

That said, I have no idea if thats the intended behaviour or not.

If you don't insight about this, I'll have a look in git blame to try to see how it got like it is now.

I'm not sure any user would have BC break if we move it after Cloud is loaded.

---

Side note for later: If we implement one of my suggestions above, i would also recommend deprecating in v6.0 serverStartComplete in favor or just having a straightforward start() or ,isReady() function we can await on.

I'll open a different issue when this issue is resolved to address this later.

[mtrezza]

From what I can see, this is a bug.

The docs say:

Callback when server has started

The server has not started if it has not loaded all of its modules. As such, if people are using this callback as intended (as you would in your case like Davi suggested), then they will currently face the issue of errors (what you originally described) because the callback is called too early.

Fixing this would not be considered a breaking change, because it is a bug fix.

My suggestion would be to fix this bug in this issue and treat the constructor change as an enhancement in a separate issue. This way we can also easily backport this fix to Parse Server 4.x.

[sadortun]

fix this bug in this issue

Ok, awesome! I'll submit a PR sometime soon ...

and treat the constructor change as an enhancement in a separate issue

Ok. Do we keep one issue with two PRs? Otherwise I'll duplicate this issue to discuss the next steps.

[mtrezza]

Yes, I suggest to split the enhancement into a separate issue and PR. This way we can fix the bug quicker and backport it to 4.x. It also allows to remodel the constructor with a deprecation (breaking change), should it be needed.

[SebC99]

I just discovered the issue.
In our case, we resolved this issue by creating a new route /healthCheck which resolves only when a boolean is set to true, which we do in serverStartComplete.
I guess the most surprising thing (which can be seen as a bug) is that the route /parse/health returns true even before serverStartComplete is called.
So a "quick" way to fix would be to make sure the /parse/health route checks the completion status before sending OK.

This lets us config AWS load balancer to check a specific route to know the instance status.

[dblythy]

If we are supporting this as a breaking change for V6, we could also remove serverStartComplete and startCallbackError and use native async/await with Error.

  const api = new ParseServer(config);
  await api.start()

[parseplatformorg]

🎉 This change has been released in version 6.0.0-alpha.16

[parseplatformorg]

🎉 This change has been released in version 6.0.0-beta.1

[parseplatformorg]

🎉 This change has been released in version 6.0.0

[sadortun]

This is amazing work guys !

My deploy scripts will be so happy 🥂