Skip to content

Conversation

fredzqm
Copy link
Contributor

@fredzqm fredzqm commented Sep 3, 2025

Previously, if a project isn't available locally:

  • No local toolkit is running
  • No code lens is available
  • Cannot start or discover emulator

This PR addressed numerous edge cases to make sure the no project experience is smooth.

When project is missing,

  • VS Code starts the toolkit, so there is error linter.
  • VS Code will show the "run local" code lens. No "run prod" because project is missing.
  • The emulator hub would still starts to set the locator file (e.g. /var/folders/6x/5fwhc57j2b10smkj7m5fgsjw00k66j/T/hub-local-demo.json). This allows VS Code to discover the projectless emulator easily.

Tested: https://screencast.googleplex.com/cast/NjAyMjU3NjcyNjg2Nzk2OHxiNzU5MDVjNC1hMA

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Summary of Changes

Hello @fredzqm, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the robustness and clarity of error handling within the VS Code extension's Data Connect integration. By introducing explicit error throwing for unresolved configurations and refining how API service paths are generated, the changes aim to provide more immediate and informative feedback when expected Data Connect configurations are missing or improperly referenced.

Highlights

  • Improved error handling for serviceId resolution: Methods like findById and findEnclosingServiceForPath now throw specific errors when a Data Connect configuration is not found, providing clearer diagnostics.
  • Enhanced robustness in API service path generation: The getApiServicePathByPath method now gracefully handles an undefined projectId by providing a default placeholder, preventing potential runtime issues.
  • Refactored servicePath logic: The servicePath method in service.ts has been streamlined to rely on the improved error handling from config.ts, reducing redundant checks and making the code more concise.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request improves error handling when resolving a service ID by throwing errors instead of returning undefined. This makes the code more robust and error paths more explicit. The changes are well-contained within ResolvedDataConnectConfigs and DataConnectService. My review includes suggestions to further enhance code clarity and type safety by removing redundant optional chaining and updating function signatures to align with the new error handling strategy.

@fredzqm fredzqm changed the title [VS Code] Improve error handling when resolving service_id [VS Code] Fix the no project FDC experience Sep 3, 2025
fredzqm and others added 4 commits September 3, 2025 10:13
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Copy link
Member

@yuchenshi yuchenshi left a comment

Choose a reason for hiding this comment

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

Let's try to agree on a single default project name. I'd say demo-local or something explicit like demo-no-project.

Right now, different values are used in different places, such as "", "p" and "local-demo". The last one may even conflict with a real project with that name. (demo-* is reserved but local-* is not)

@github-project-automation github-project-automation bot moved this to Changes Requested [PR] in [Cloud] Extensions + Functions Sep 3, 2025
@fredzqm
Copy link
Contributor Author

fredzqm commented Sep 3, 2025

Let's try to agree on a single default project name. I'd say demo-local or something explicit like demo-no-project.

Right now, different values are used in different places, such as "", "p" and "local-demo". The last one may even conflict with a real project with that name. (demo-* is reserved but local-* is not)

What's your recommendation? I don't mind any of them. Just want this to work

BTW, project doesn't matter for FDC emulator. We key based on just service_id.

@fredzqm
Copy link
Contributor Author

fredzqm commented Sep 3, 2025

@yuchenshi

What do you think? There is no project. The only thing that needs to be specified is the discovery file when project is missing.

  let filename = `hub_no_project.json`;
    if (projectId) {
      filename = `hub-${projectId}.json`;
    }

Better?

@fredzqm fredzqm changed the title [VS Code] Fix the no project FDC experience [VS Code] Fix the no project experience Sep 3, 2025
@fredzqm fredzqm changed the title [VS Code] Fix the no project experience [VS Code] Fix the no project emulator experience Sep 3, 2025
@yuchenshi
Copy link
Member

@fredzqm I recommend demo-no-project because it will be easier to tell or debug in logs or support cases later. I believe a demo project ID works better than the empty string. A few emulators like Auth and UI have troubles with no project ID and will either be skipped or throw an error. Instead of coding branching logic everywhere downstream, a fake project ID should work out of the box.

Copy link
Contributor

@joehan joehan left a comment

Choose a reason for hiding this comment

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

Let's just have this default to demo-no-project

@fredzqm
Copy link
Contributor Author

fredzqm commented Sep 3, 2025

SGTM. Just changed the PR to use demo-no-project

const serviceId = dataConnectConfig?.value.serviceId;
const locationId = dataConnectConfig?.dataConnectLocation;
return `projects/${projectId}/locations/${locationId}/services/${serviceId}`;
return `projects/${projectId || "p"}/locations/${locationId}/services/${serviceId}`;
Copy link
Member

Choose a reason for hiding this comment

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

Do we still need this? If so, I'd suggest

Suggested change
return `projects/${projectId || "p"}/locations/${locationId}/services/${serviceId}`;
return `projects/${projectId || "demo-ignored"}/locations/${locationId}/services/${serviceId}`;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, when .firebaserc is missing. project is undefined here.

Sure, I will use the no-project placeholder.

Copy link
Member

Choose a reason for hiding this comment

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

Just add a comment that the Data Connect emulator ignores the project ID and will return the right response pretending that there's only one project

host: pubsubAddr.host,
port: pubsubAddr.port,
projectId,
projectId: projectId,
Copy link
Member

Choose a reason for hiding this comment

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

revert this

}

const hubClient = new EmulatorHubClient(projectId);
const hubClient = new EmulatorHubClient(projectId || "");
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const hubClient = new EmulatorHubClient(projectId || "");
const hubClient = new EmulatorHubClient(projectId);

@fredzqm fredzqm enabled auto-merge (squash) September 4, 2025 00:29
@fredzqm fredzqm requested a review from joehan September 4, 2025 04:45
@fredzqm
Copy link
Contributor Author

fredzqm commented Sep 4, 2025

@joehan I think this PR still need a LGTM from you because there was a "requested change" before.

@@ -1,0 +1 @@
- `firebase emulator:start` use a default project if no project can be found. (#9072)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- `firebase emulator:start` use a default project if no project can be found. (#9072)
- Changed `firebase emulator:start` to use a default project `demo-no-project` if no project can be found. (#9072)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Err, my bad. Shouldn't have enable auto-merge.

@fredzqm fredzqm merged commit bbed1b5 into master Sep 4, 2025
44 of 48 checks passed
@fredzqm fredzqm deleted the fz/service-path branch September 4, 2025 20:31
@github-project-automation github-project-automation bot moved this from Changes Requested [PR] to Done in [Cloud] Extensions + Functions Sep 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants