Skip to content

Commit

Permalink
🏗 Clean up all gulp style error throwing in build-system (#33447)
Browse files Browse the repository at this point in the history
  • Loading branch information
rsimha committed Mar 23, 2021
1 parent 7dac0bb commit 474290f
Show file tree
Hide file tree
Showing 9 changed files with 19 additions and 73 deletions.
11 changes: 0 additions & 11 deletions build-system/global.d.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,4 @@
declare global {
// TODO(#28387) delete this once all uses of these properties have been removed.
interface Error {
/**
* In the build-system, Error objects contain useful info like underlying
* stack traces in the message field, so we print that and hide the less
* useful nodejs stack.
*/
showStack?: boolean;
status?: string;
}

interface CompilerNode {
type: string;
name: string;
Expand Down
4 changes: 1 addition & 3 deletions build-system/server/app-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,7 @@ function setServeMode(modeOptions) {
if (isRtvMode(rtv)) {
serveMode = rtv;
} else {
const err = new Error(`Invalid rtv: ${rtv}. (Must be 15 digits long.)`);
err.showStack = false;
throw err;
throw new Error(`Invalid rtv: ${rtv}. (Must be 15 digits long.)`);
}
}
}
Expand Down
10 changes: 1 addition & 9 deletions build-system/tasks/check-exact-versions.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ const checkerExecutable = 'npx npm-exact-versions';
* @return {!Promise}
*/
async function checkExactVersions() {
let success = true;
const packageJsonFiles = globby.sync(['**/package.json', '!**/node_modules']);
packageJsonFiles.forEach((file) => {
const checkerCmd = `${checkerExecutable} --path ${file}`;
Expand All @@ -41,7 +40,7 @@ async function checkExactVersions() {
'do not have an exact version.'
);
logWithoutTimestamp(gitDiffFileMaster(file));
success = false;
throw new Error('Check failed');
} else {
logLocalDev(
green('SUCCESS:'),
Expand All @@ -51,13 +50,6 @@ async function checkExactVersions() {
);
}
});
if (success) {
return Promise.resolve();
} else {
const reason = new Error('Check failed');
reason.showStack = false;
return Promise.reject(reason);
}
}

module.exports = {
Expand Down
27 changes: 8 additions & 19 deletions build-system/tasks/check-sourcemaps.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,6 @@ const sourcemapUrlMatcher =
const expectedFirstLineFile = 'src/polyfills/abort-controller.js'; // First file that is compiled into v0.js.
const expectedFirstLineCode = 'class AbortController {'; // First line of code in that file.

/**
* Throws an error with the given message
*
* @param {string} message
*/
function throwError(message) {
const err = new Error(message);
err.showStack = false;
throw err;
}

/**
* Build runtime with sourcemaps if needed.
*/
Expand All @@ -68,7 +57,7 @@ function maybeBuild() {
function getSourcemapJson(map) {
if (!fs.existsSync(map)) {
log(red('ERROR:'), 'Could not find', cyan(map));
throwError(`Could not find sourcemap file '${map}'`);
throw new Error(`Could not find sourcemap file '${map}'`);
}
return JSON.parse(fs.readFileSync(map, 'utf8'));
}
Expand All @@ -83,11 +72,11 @@ function checkSourcemapUrl(sourcemapJson, map) {
log('Inspecting', cyan('sourceRoot'), 'in', cyan(map) + '...');
if (!sourcemapJson.sourceRoot) {
log(red('ERROR:'), 'Could not find', cyan('sourceRoot'));
throwError('Could not find sourcemap URL');
throw new Error('Could not find sourcemap URL');
}
if (!sourcemapJson.sourceRoot.match(sourcemapUrlMatcher)) {
log(red('ERROR:'), cyan(sourcemapJson.sourceRoot), 'is badly formatted');
throwError('Badly formatted sourcemap URL');
throw new Error('Badly formatted sourcemap URL');
}
}

Expand All @@ -101,7 +90,7 @@ function checkSourcemapSources(sourcemapJson, map) {
log('Inspecting', cyan('sources'), 'in', cyan(map) + '...');
if (!sourcemapJson.sources) {
log(red('ERROR:'), 'Could not find', cyan('sources'));
throwError('Could not find sources array');
throw new Error('Could not find sources array');
}
const invalidSources = sourcemapJson.sources
.filter((source) => !source.match(/\[.*\]/)) // Ignore non-path sources '[...]'
Expand All @@ -113,7 +102,7 @@ function checkSourcemapSources(sourcemapJson, map) {
cyan('sources') + ':',
cyan(invalidSources.join(', '))
);
throwError('Invalid paths in sources array');
throw new Error('Invalid paths in sources array');
}
}

Expand All @@ -138,7 +127,7 @@ function checkSourcemapMappings(sourcemapJson, map) {
log('Inspecting', cyan('mappings'), 'in', cyan(map) + '...');
if (!sourcemapJson.mappings) {
log(red('ERROR:'), 'Could not find', cyan('mappings'));
throwError('Could not find mappings array');
throw new Error('Could not find mappings array');
}

// Zeroth sub-array corresponds to ';' and has no mappings.
Expand All @@ -158,14 +147,14 @@ function checkSourcemapMappings(sourcemapJson, map) {
log('Actual:', cyan(firstLineFile));
log('Expected:', cyan(expectedFirstLineFile));
log(helpMessage);
throwError('Found mapping for incorrect file');
throw new Error('Found mapping for incorrect file');
}
if (firstLineCode != expectedFirstLineCode) {
log(red('ERROR:'), 'Found mapping for incorrect code.');
log('Actual:', cyan(firstLineCode));
log('Expected:', cyan(expectedFirstLineCode));
log(helpMessage);
throwError('Found mapping for incorrect code');
throw new Error('Found mapping for incorrect code');
}
}

Expand Down
12 changes: 3 additions & 9 deletions build-system/tasks/cherry-pick.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,14 +84,10 @@ async function cherryPick() {
let onto = String(argv.onto || '');

if (!commits.length) {
const error = new Error('Must provide commit list with --commits');
error.showStack = false;
throw error;
throw new Error('Must provide commit list with --commits');
}
if (!onto) {
const error = new Error('Must provide 13-digit AMP version with --onto');
error.showStack = false;
throw error;
throw new Error('Must provide 13-digit AMP version with --onto');
}
if (onto.length === 15) {
log(
Expand All @@ -103,9 +99,7 @@ async function cherryPick() {
onto = onto.substr(2);
}
if (onto.length !== 13) {
const error = new Error('Expected 13-digit AMP version');
error.showStack = false;
throw error;
throw new Error('Expected 13-digit AMP version');
}

const branch = cherryPickBranchName(onto, commits.length);
Expand Down
4 changes: 1 addition & 3 deletions build-system/tasks/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -404,9 +404,7 @@ function handleBundleError(err, continueOnError, destFilename) {
if (continueOnError) {
log(red('ERROR:'), reasonMessage);
} else {
const reason = new Error(reasonMessage);
reason.showStack = false;
throw reason;
throw new Error(reasonMessage);
}
}

Expand Down
7 changes: 2 additions & 5 deletions build-system/tasks/pr-check.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,13 @@ const jobName = 'pr-check.js';
/**
* This file runs tests against the local workspace to mimic the CI build as
* closely as possible.
* @param {Function} cb
*/
async function prCheck(cb) {
async function prCheck() {
setLoggingPrefix(jobName);

const failTask = () => {
stopTimer(jobName, startTime);
const err = new Error('Local PR check failed. See logs above.');
err.showStack = false;
cb(err);
throw new Error('Local PR check failed. See logs above.');
};

const runCheck = (cmd) => {
Expand Down
4 changes: 1 addition & 3 deletions build-system/tasks/server-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,9 +162,7 @@ function reportResult() {
`(${cyan(passed)} passed, ${cyan(failed)} failed).`;
if (failed > 0) {
log(red('ERROR:'), result);
const err = new Error('Tests failed');
err.showStack = false;
throw err;
throw new Error('Tests failed');
} else {
log(green('SUCCESS:'), result);
}
Expand Down
13 changes: 2 additions & 11 deletions build-system/tasks/storybook/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,6 @@ const repoDir = path.join(__dirname, '../../..');
*/
const envConfigDir = (env) => path.join(__dirname, `${env}-env`);

/**
* @param {string} message Message for amp task (call stack is already in logs)
*/
const throwError = (message) => {
const err = new Error(message);
err.showStack = false;
throw err;
};

/**
* @param {string} env 'amp' or 'preact'
*/
Expand All @@ -67,7 +58,7 @@ function launchEnv(env) {
].join(' '),
{cwd: __dirname, stdio: 'inherit'}
).on('error', () => {
throwError('Launch failed');
throw new Error('Launch failed');
});
}

Expand Down Expand Up @@ -103,7 +94,7 @@ function buildEnv(env) {
{cwd: __dirname, stdio: 'inherit'}
);
if (result.status != 0) {
throwError('Build failed');
throw new Error('Build failed');
}
}

Expand Down

0 comments on commit 474290f

Please sign in to comment.