Skip to content
This repository was archived by the owner on May 2, 2022. It is now read-only.

Improve error handling of Rollup #75

Merged
merged 2 commits into from
Sep 25, 2020
Merged

Improve error handling of Rollup #75

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ccfdb3c
Improve error handling of Rollup
ehmicky Sep 25, 2020
7bcf5b0
Add comment
ehmicky Sep 25, 2020
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
22 changes: 14 additions & 8 deletions src/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,14 +123,20 @@ async function bundleFunctions(file, utils) {
},
};

const bundle = await rollup.rollup(options);
const {
output: [{ code }],
} = await bundle.generate({
format: "iife",
compact: true,
});
return code;
try {
const bundle = await rollup.rollup(options);
const {
output: [{ code }],
} = await bundle.generate({
format: "iife",
compact: true,
});
return code;
} catch (error) {
// This will stop the execution of this plugin.
// No Edge handlers will be uploaded.
return utils.build.failBuild("Error while bundling Edge handlers", { error });
Copy link
Contributor

@mraerino mraerino Sep 25, 2020

Choose a reason for hiding this comment

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

will this still throw? what will the return value of the function become? don't we have to cancel upstream steps somehow?

Copy link
Contributor Author

@ehmicky ehmicky Sep 25, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

From a plugin authoring standpoint, the execution will stop right there. The return is actually not needed (and it could be removed).
The way this is implemented under the hood is by throwing an error.

Rollup should stop since it just failed. Also, uploading has not started yet. The build will stop, which should prevent anything else to be run. Is there something specific you had in mind that should be cancelled?

Copy link
Contributor

@mraerino mraerino Sep 25, 2020

Choose a reason for hiding this comment

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

i just wanted to make sure we don't continue uploading empty code when returning here

Copy link
Contributor

@mraerino mraerino Sep 25, 2020

Choose a reason for hiding this comment

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

can you add a comment that calling this actually throws and thereby stops execution?

Copy link
Contributor Author

@ehmicky ehmicky Sep 25, 2020

Choose a reason for hiding this comment

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

Just did 👍

}
}

/**
Expand Down
2 changes: 2 additions & 0 deletions test/fixtures/syntax-error/edge-handlers/example.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// Intentional JavaScript error
exportt function onRequest() {}
2 changes: 2 additions & 0 deletions test/fixtures/syntax-error/netlify.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
[[plugins]]
package = "../../.."
6 changes: 6 additions & 0 deletions test/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ const FIXTURES_DIR = `${__dirname}/fixtures`;
const INTEGRATION_TEST_DIR = `${FIXTURES_DIR}/integration-test`;
const CONFIG_FIXTURE_DIR = `${FIXTURES_DIR}/config-dir`;
const WRONG_CONFIG_FIXTURE_DIR = `${FIXTURES_DIR}/wrong-config-dir`;
const SYNTAX_ERROR_FIXTURE_DIR = `${FIXTURES_DIR}/syntax-error`;

test("Edge handlers should be bundled", async (t) => {
await runNetlifyBuild(t, INTEGRATION_TEST_DIR);
Expand Down Expand Up @@ -35,3 +36,8 @@ test("Edge handlers directory build.edge_handlers misconfiguration is reported",
const { output } = await runNetlifyBuild(t, WRONG_CONFIG_FIXTURE_DIR, { expectedSuccess: false });
t.true(output.includes("does-not-exist"));
});

test("Edge handlers directory build.edge_handlers syntax error is reported", async (t) => {
const { output } = await runNetlifyBuild(t, SYNTAX_ERROR_FIXTURE_DIR, { expectedSuccess: false });
t.true(output.includes("Error while bundling"));
});