Make vector warnings best effort #7241
Conversation
| } | ||
| } catch (err: any) { |
There was a problem hiding this comment.
I don't think this try-catch is needed.
If fdc build returns non zero exit code, it failed to read sources here.
Compile errors are still zero exit code with gql error part of output json.
Do we have a repro of this bug?
There was a problem hiding this comment.
If you look into the implementation of .build below, any output on stderr will be considered fatal, including verbose logs. This may be overkill. Can we agree on the desirable behavior? I think most tools have the main output (e.g. JSON) on stdout and logging should be done freely on stderr
There was a problem hiding this comment.
I agree.
We should throw err based on exit code. Stderr goes in the error message in case of failure, but otherwise debug logs
There was a problem hiding this comment.
Fair points - changed build to only throw on nonzero error codes. I do still think this should be a best effort check in this context, so I think it makes sense to keep this try/catch.
| } | ||
| return start(Emulators.DATACONNECT, { | ||
| ...this.args, |
There was a problem hiding this comment.
While testing, I noticed this was leading to a bunch of 'Ignored unrecognized arg' debug logs, so I cleaned it up.
There was a problem hiding this comment.
I think we should keep auto_download. It's probably not needed in this context because .build happens to download the binary, but this probably won't survive a refactor or two.
| @@ -295,6 +295,10 @@ const Commands: { [s in DownloadableEmulators]: DownloadableEmulatorCommand } = | |||
| "local_connection_string", | |||
| "project_id", | |||
| "service_location", | |||
| "disable_sdk_generation", | |||
There was a problem hiding this comment.
Somewhat unrelated, but updated this list of flags to match the binary.
| } | ||
| return start(Emulators.DATACONNECT, { | ||
| ...this.args, |
There was a problem hiding this comment.
I think we should keep auto_download. It's probably not needed in this context because .build happens to download the binary, but this probably won't survive a refactor or two.
Co-authored-by: Yuchen Shi <yuchenshi@google.com>
Description
Catch any errors from 'fdc build' in this context so that we always try to start the emulator.