Skip to content
Draft
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
20 changes: 20 additions & 0 deletions src/deploy/apphosting/util.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import * as path from "path";
import * as tmp from "tmp";
import * as tar from "tar";
import * as util from "./util";
import { FirebaseError } from "../../error";

describe("util", () => {
let tmpDir: tmp.DirResult;
Expand Down Expand Up @@ -78,5 +79,24 @@ describe("util", () => {
expect(files).to.include("dist/index.js");
expect(files).to.not.include("apphosting.yaml");
});

it("should throw an error if local build archive size exceeds limit", async () => {
fs.writeFileSync(path.join(distDir, "index.js"), "console.log('hello')");

const config = {
backendId: "test-backend",
rootDir: "",
ignore: [],
localBuild: true,
};

try {
await util.createLocalBuildTarArchive(config, rootDir, path.relative(rootDir, distDir), 1);
expect.fail("Should have thrown an error");
} catch (err: unknown) {
expect(err).to.be.instanceOf(FirebaseError);
expect((err as FirebaseError).message).to.match(/The final build artifact is larger than/);
}
});
});
});
11 changes: 11 additions & 0 deletions src/deploy/apphosting/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
config: AppHostingSingle,
rootDir: string,
targetSubDir?: string,
sizeLimitBytes: number = 250 * 1024 * 1024,
Comment thread
falahat marked this conversation as resolved.
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

The value 250 * 1024 * 1024 is a magic number representing the Cloud Run size limit. It should be defined as a named constant (e.g., CLOUD_RUN_SIZE_LIMIT_BYTES) at the top of the file to improve maintainability and clarity, especially since this limit is referenced in the error message logic as well.

): Promise<string> {
const tmpFile = tmp.fileSync({ prefix: `${config.backendId}-`, postfix: ".tar.gz" }).name;

Expand Down Expand Up @@ -69,6 +70,16 @@
},
allFiles,
);

const stats = fs.statSync(tmpFile);
if (config.localBuild && stats.size > sizeLimitBytes) {
const sizeInMB = stats.size / (1024 * 1024);
const limitInMB = sizeLimitBytes / (1024 * 1024);
throw new FirebaseError(
`The final build artifact is larger than ${limitInMB.toFixed(0)}MB (current size: ${sizeInMB.toFixed(2)}MB). Please reduce the size of your build artifacts.`,
);
}

return tmpFile;
}

Expand Down Expand Up @@ -110,7 +121,7 @@
await pipeAsync(archive, fileStream);
} catch (err: unknown) {
throw new FirebaseError(
`Could not read source directory. Remove links and shortcuts and try again. Original: ${err}`,

Check warning on line 124 in src/deploy/apphosting/util.ts

View workflow job for this annotation

GitHub Actions / lint (20)

Invalid type "unknown" of template literal expression
{ original: err as Error, exit: 1 },
);
}
Expand Down
Loading