diff --git a/src/dataconnect/build.ts b/src/dataconnect/build.ts index bd67e0760a0..2dfe7e8f832 100644 --- a/src/dataconnect/build.ts +++ b/src/dataconnect/build.ts @@ -4,7 +4,7 @@ import { FirebaseError } from "../error"; import * as experiments from "../experiments"; import { promptOnce } from "../prompt"; import * as utils from "../utils"; -import { prettify, prettifyWithWorkaround } from "./graphqlError"; +import { prettify, prettifyTable } from "./graphqlError"; import { DeploymentMetadata, GraphqlError } from "./types"; export async function build( @@ -44,10 +44,11 @@ export async function handleBuildErrors( const requiredForces = errors.filter((w) => w.extensions?.warningLevel === "REQUIRE_FORCE"); if (requiredForces.length && !force) { + // Only INACCESSIBLE issues fall in this category. utils.logLabeledError( "dataconnect", `There are changes in your schema or connectors that will result in broken behavior:\n` + - prettifyWithWorkaround(requiredForces), + prettifyTable(requiredForces), ); throw new FirebaseError("Rerun this command with --force to deploy these changes."); } @@ -59,18 +60,19 @@ export async function handleBuildErrors( { name: "Reject changes and abort", value: "abort" }, ]; if (requiredAcks.length) { + // This category contains BREAKING and INSECURE issues. utils.logLabeledWarning( "dataconnect", - `There are changes in your schema or connectors that may break your existing applications. These changes require explicit acknowledgement to proceed. You may either reject the changes and update your sources with the suggested workaround(s), if any, or acknowledge these changes and proceed with the deployment:\n` + - prettifyWithWorkaround(requiredAcks), + `There are changes in your schema or connectors that may break your existing applications or introduce operations that are insecure. These changes require explicit acknowledgement to proceed. You may either reject the changes and update your sources with the suggested workaround(s), if any, or acknowledge these changes and proceed with the deployment:\n` + + prettifyTable(requiredAcks), ); if (nonInteractive && !force) { throw new FirebaseError( - "Explicit acknowledgement required for breaking schema or connector changes. Rerun this command with --force to deploy these changes.", + "Explicit acknowledgement required for breaking schema or connector changes and new insecure operations. Rerun this command with --force to deploy these changes.", ); } else if (!nonInteractive && !force && !dryRun) { const result = await promptOnce({ - message: "Would you like to proceed with these breaking changes?", + message: "Would you like to proceed with these changes?", type: "list", choices, default: "abort", @@ -81,10 +83,11 @@ export async function handleBuildErrors( } } if (interactiveAcks.length) { + // This category contains WARNING and EXISTING_INSECURE issues. utils.logLabeledWarning( "dataconnect", - `There are changes in your schema or connectors that may cause unexpected behavior in your existing applications:\n` + - interactiveAcks.map(prettify).join("\n"), + `There are existing insecure operations or changes in your schema or connectors that may cause unexpected behavior in your existing applications:\n` + + prettifyTable(interactiveAcks), ); if (!nonInteractive && !force && !dryRun) { const result = await promptOnce({ diff --git a/src/dataconnect/graphqlError.ts b/src/dataconnect/graphqlError.ts index 5f48a892102..8fbf7d8d954 100644 --- a/src/dataconnect/graphqlError.ts +++ b/src/dataconnect/graphqlError.ts @@ -13,23 +13,35 @@ export function prettify(err: GraphqlError): string { return header.length ? `${header}: ${message}` : message; } -export function prettifyWithWorkaround(errs: GraphqlError[]): string { +function splitIssueMessage(err: GraphqlError): string[] { + const msg = err.message.split(": "); + if (msg.length >= 2) { + return [msg[0], msg.slice(1).join(":")]; + } + return ["", err.message]; +} + +export function prettifyTable(errs: GraphqlError[]): string { const table = new Table({ - head: ["Issue", "Workaround", "Reason"], + head: ["Type", "Issue", "Workaround", "Reason"], style: { head: ["yellow"] }, - colWidths: [50, 50, 50], + colWidths: [20, 50, 50, 50], wordWrap: true, }); + // We want to present BREAKING before INSECURE changes. Ordering of other issues matters less, but we want to keep categories grouped together. + errs.sort((a, b) => a.message.localeCompare(b.message)); for (const e of errs) { + const msg = splitIssueMessage(e); + e.message = msg[1]; if (!e.extensions?.workarounds?.length) { - table.push([prettify(e), "", ""]); + table.push([msg[0], prettify(e), "", ""]); } else { const workarounds = e.extensions.workarounds; for (let i = 0; i < workarounds.length; i++) { if (i === 0) { - table.push([prettify(e), workarounds[i].description, workarounds[i].reason]); + table.push([msg[0], prettify(e), workarounds[i].description, workarounds[i].reason]); } else { - table.push(["", workarounds[i].description, workarounds[i].reason]); + table.push(["", "", workarounds[i].description, workarounds[i].reason]); } } }