Skip to content

Update formatting of connector evolution and insecure operation issues. #8204

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.

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

Merged
merged 7 commits into from
Feb 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions src/dataconnect/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@
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(

Check warning on line 10 in src/dataconnect/build.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing JSDoc comment
options: Options,
configDir: string,
dryRun?: boolean,
Expand All @@ -29,7 +29,7 @@
return buildResult?.metadata ?? {};
}

export async function handleBuildErrors(

Check warning on line 32 in src/dataconnect/build.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing return type on function

Check warning on line 32 in src/dataconnect/build.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing JSDoc comment
errors: GraphqlError[],
nonInteractive: boolean,
force: boolean,
Expand All @@ -44,10 +44,11 @@

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.");
}
Expand All @@ -59,18 +60,19 @@
{ 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({

Check warning on line 74 in src/dataconnect/build.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe assignment of an `any` value
message: "Would you like to proceed with these breaking changes?",
message: "Would you like to proceed with these changes?",
type: "list",
choices,
default: "abort",
Expand All @@ -81,13 +83,14 @@
}
}
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({

Check warning on line 93 in src/dataconnect/build.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Unsafe assignment of an `any` value
message: "Would you like to proceed with these changes?",
type: "list",
choices,
Expand Down
24 changes: 18 additions & 6 deletions src/dataconnect/graphqlError.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { GraphqlError } from "./types";
import * as Table from "cli-table3";

export function prettify(err: GraphqlError): string {

Check warning on line 4 in src/dataconnect/graphqlError.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing JSDoc comment
const message = err.message;
let header = err.extensions?.file ?? "";
if (err.locations && err.locations.length) {
Expand All @@ -13,23 +13,35 @@
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 {

Check warning on line 24 in src/dataconnect/graphqlError.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Missing JSDoc comment
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]);
}
}
}
Expand Down
Loading