Skip to content

chore(firestore-bigquery-export): improve fs-bq-schema-views when bigQueryID is invalid (rebased)#2782

Draft
IzaakGough wants to merge 1 commit intonextfrom
rebased-@invertase/improve-invalid-projectid-err-msg
Draft

chore(firestore-bigquery-export): improve fs-bq-schema-views when bigQueryID is invalid (rebased)#2782
IzaakGough wants to merge 1 commit intonextfrom
rebased-@invertase/improve-invalid-projectid-err-msg

Conversation

@IzaakGough
Copy link
Copy Markdown
Contributor

@IzaakGough IzaakGough commented Apr 20, 2026

Fixes #2540

supersedes #2594

Copy link
Copy Markdown
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 in the gen-schema-view script by providing more descriptive messages for invalid BigQuery Project IDs and removes the onStartResize event from the storage-resize-images extension. Key feedback includes addressing a redundant configuration parsing call that could cause duplicate prompts, improving error logging by avoiding JSON.stringify on Error objects, and ensuring the process exits with a failure code on error. Additionally, the removal of event triggers should be synchronized with the event definitions in extension.yaml to maintain consistency.

Comment on lines +77 to +81
parseConfig()
.then((parsedConfig) => {
config = parsedConfig;
return run();
})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

Calling parseConfig() here is redundant because the run() function (called on line 80) also invokes await parseConfig() internally. This results in the configuration being parsed twice, which will cause duplicate interactive prompts if the script is run in interactive mode. Consider refactoring the run function to accept the configuration as an optional parameter to avoid this duplication.

}
}

console.log(JSON.stringify(error));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

JSON.stringify(error) often returns an empty object {} for standard JavaScript Error objects because their properties (like message and stack) are non-enumerable. Additionally, it can throw an error if the object contains circular references. Consider logging the error object directly with console.error(error) to ensure all details, including the stack trace, are visible.


console.log(JSON.stringify(error));
console.error(error.message);
process.exit();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

When an error occurs, the process should exit with a non-zero status code to indicate failure to the calling shell or CI environment.

Suggested change
process.exit();
process.exit(1);

Comment thread storage-resize-images/functions/src/index.ts
@IzaakGough IzaakGough requested a review from cabljac April 20, 2026 13:58
@IzaakGough IzaakGough changed the title Rebased @invertase/improve invalid projectid err msg chore(firestore-bigquery-export): improve fs-bq-schema-views when bigQueryID is invalid (rebased) Apr 20, 2026
@IzaakGough IzaakGough force-pushed the rebased-@invertase/improve-invalid-projectid-err-msg branch from cb558ba to baa730a Compare April 21, 2026 14:29
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.

🐛 [fs-bq-schema-views] Getting "ProjectId must be non-empty" HTTP error

3 participants