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.

Sign up for GitHub

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

Jump to bottom

Fix error message for port conflict #1433

Merged
merged 3 commits into from
Jun 26, 2019
Merged

Fix error message for port conflict #1433

merged 3 commits into from
Jun 26, 2019

Conversation

samtstern
Copy link
Contributor

@samtstern samtstern commented Jun 24, 2019 •
edited
Loading

Description

Fixes #1432
Fixes #1032

Scenarios Tested

Before

$ firebase serve --only firestore
i  firestore: Logging to firestore-debug.log
✔  firestore: Emulator started at http://localhost:8080

Error: firestore: emulator has exited with code: 1

After

$ npx firebase serve --only firestore

Error: Port 8080 is not open, could not start firestore emulator.

Sample Commands

N/A

@googlebot googlebot added the cla: yes Manual indication that this has passed CLA. label Jun 24, 2019
@samtstern samtstern requested a review from abeisgoat June 24, 2019 17:34
samtstern
samtstern commented Jun 24, 2019
View reviewed changes
src/emulator/registry.ts
await instance.start();
this.set(instance.getName(), instance);
await controller.waitForPortClosed(instance.getInfo().port);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also moved the waitForPortClosed() bit into the registry so that we only mark an emulator as started once it takes its port.

@coveralls
Copy link

coveralls commented Jun 24, 2019 •
edited
Loading

Coverage Status

Coverage increased (+0.04%) to 62.073% when pulling b1199b1 on ss-fix-1432 into 729083f on master.

@samtstern samtstern mentioned this pull request Jun 25, 2019
Closed
abeisgoat
abeisgoat approved these changes Jun 26, 2019
View reviewed changes
Copy link
Contributor

@abeisgoat abeisgoat left a comment

Choose a reason for hiding this comment

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

LGTM, also does solve #1032

@samtstern samtstern merged commit a00f1bb into master Jun 26, 2019
@samtstern samtstern deleted the ss-fix-1432 branch June 26, 2019 17:34
This was referenced Jul 3, 2019
Merged
Merged
@snyk-bot snyk-bot mentioned this pull request Jan 20, 2022
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@abeisgoat abeisgoat abeisgoat approved these changes

Assignees
No one assigned
Labels
cla: yes Manual indication that this has passed CLA.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Firestore emulator crashes Display a dedicated errors when the restPort is already used
4 participants
@samtstern @coveralls @abeisgoat @googlebot