Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default timeout is 60s + user define custom timeout per emulator #3482

Closed

Conversation

erikologic
Copy link
Contributor

Description

Fix #2379
Firestore is failing to start in the hardcoded timeout of 30 secs.
This PR:

  • increase the default timeout to 60 secs
  • leave open to the user the configuration of the timeout for each emulator in the same pattern of host and port

Scenarios Tested

GIVEN a user does not provide a timeout
WHEN firebase emulators:start is called
THEN firestore emulator will fail to run with Error: TIMEOUT: Port 8081 on localhost was not active within 60000ms

GIVEN a user provides a timeout in firebase.json for firestore of 10ms
WHEN firebase emulators:start --only firestore is called
THEN firestore emulator will fail to run with Error: TIMEOUT: Port 8081 on localhost was not active within 10ms

GIVEN a user provides a timeout in firebase.json for hosting of 10ms
WHEN firebase emulators:start --only firestore is called
THEN firestore emulator will fail to run with Error: TIMEOUT: Port 8081 on localhost was not active within 60000ms*

*I understand these is a bit flacky - couldn't think any better at midnight 😛
I actually tested them with the old value of 30 secs

Sample Commands

In firebase.json:

{
  //...
  "emulators": {
    "auth": {
      "port": 9099
    },
    "firestore": {
      "timeout": 60000,
      "port": 8081
    },
    "hosting": {
      "port": 5000
    },
    "ui": {
      "enabled": true
    }
  }
}

@google-cla google-cla bot added the cla: yes Manual indication that this has passed CLA. label Jun 10, 2021
@@ -311,7 +314,10 @@ function findExportMetadata(importPath: string): ExportMetadata | undefined {
}
}

export async function startAll(options: any, showUI: boolean = true): Promise<void> {
/**
Copy link
Contributor Author

@erikologic erikologic Jun 10, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

linting no-sense...

@samtstern
Copy link
Contributor

@mrenrich84 thanks for this! I am in favor of increasing the timeout to 60s but not yet convinced that adding a timeout parameter to the json is the right move. Plus even if it is, we have an internal review process for all API / configuration changes and we'd have to go through that.

Would you mind reducing this PR to just the 30s --> 60s change and then we can have a separate discussion about the timeout configuration?

@erikologic
Copy link
Contributor Author

erikologic commented Jun 11, 2021

😆 😆 😆
Was thinking about that all the time...
What about an env variable to control that timeout value?

@samtstern
Copy link
Contributor

@mrenrich84 I think an env var brings up the same discussion. I'd like to think about a few things before moving forward with more configuration:

  • Is there a way we can change the behavior of Firestore imports so that they don't cause timeouts at all?
  • If we need timeout configuration, do we need per-emulator timeout configs or should it be global?

@erikologic
Copy link
Contributor Author

There are few conflicts with the main branch and I'm too lazy to get lost in merge/rebase now 😛
Opened a new PR for just hardcoding the timeout to 60 secs.

@erikologic erikologic closed this Jun 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Manual indication that this has passed CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error: TIMEOUT: Port 8080 on localhost was not active within 30000ms
2 participants