Skip to content

Commit

Permalink
fix: set customErrorInfo on zisi/esbuild errors (#700)
Browse files Browse the repository at this point in the history
* fix: set `customErrorInfo` on zisi/esbuild errors

* chore: fix test

* chore: add test

* chore: remove .only

* refactor: pass functionName to customErrorInfo

* chore: update test
  • Loading branch information
eduardoboucas committed Oct 6, 2021
1 parent 55b5529 commit b2bf035
Show file tree
Hide file tree
Showing 9 changed files with 70 additions and 21 deletions.
22 changes: 18 additions & 4 deletions src/node_dependencies/index.js
Expand Up @@ -29,13 +29,14 @@ const listFilesUsingLegacyBundler = async function ({
featureFlags,
srcPath,
mainFile,
name,
srcDir,
stat,
pluginsModulesPath,
}) {
const [treeFiles, depFiles] = await Promise.all([
getTreeFiles(srcPath, stat),
getDependencies(mainFile, srcDir, pluginsModulesPath, featureFlags),
getDependencies({ featureFlags, functionName: name, mainFile, pluginsModulesPath, srcDir }),
])
const files = [...treeFiles, ...depFiles].map(normalize)
const uniqueFiles = [...new Set(files)]
Expand All @@ -52,12 +53,19 @@ const isNotJunk = function (file) {
}

// Retrieve all the files recursively required by a Node.js file
const getDependencies = async function (mainFile, srcDir, pluginsModulesPath, featureFlags) {
const getDependencies = async function ({ featureFlags, functionName, mainFile, pluginsModulesPath, srcDir }) {
const packageJson = await getPackageJson(srcDir)
const state = getNewCache()

try {
return await getFileDependencies({ featureFlags, path: mainFile, packageJson, pluginsModulesPath, state })
return await getFileDependencies({
featureFlags,
functionName,
path: mainFile,
packageJson,
pluginsModulesPath,
state,
})
} catch (error) {
error.message = `In file "${mainFile}"\n${error.message}`
throw error
Expand All @@ -66,6 +74,7 @@ const getDependencies = async function (mainFile, srcDir, pluginsModulesPath, fe

const getFileDependencies = async function ({
featureFlags,
functionName,
path,
packageJson,
pluginsModulesPath,
Expand All @@ -80,14 +89,15 @@ const getFileDependencies = async function ({

const basedir = dirname(path)
const dependencies = featureFlags.parseWithEsbuild
? await listImports({ path })
? await listImports({ functionName, path })
: precinct.paperwork(path, { includeCore: false })
const depsPaths = await Promise.all(
dependencies.filter(Boolean).map((dependency) =>
getImportDependencies({
dependency,
basedir,
featureFlags,
functionName,
packageJson,
pluginsModulesPath,
state,
Expand All @@ -104,6 +114,7 @@ const getImportDependencies = function ({
dependency,
basedir,
featureFlags,
functionName,
packageJson,
pluginsModulesPath,
state,
Expand All @@ -115,6 +126,7 @@ const getImportDependencies = function ({
dependency,
basedir,
featureFlags,
functionName,
packageJson,
pluginsModulesPath,
state,
Expand All @@ -134,6 +146,7 @@ const getTreeShakedDependencies = async function ({
dependency,
basedir,
featureFlags,
functionName,
packageJson,
pluginsModulesPath,
state,
Expand All @@ -142,6 +155,7 @@ const getTreeShakedDependencies = async function ({
const path = await resolvePathPreserveSymlinks(dependency, [basedir, pluginsModulesPath].filter(Boolean))
const depsPath = await getFileDependencies({
featureFlags,
functionName,
path,
packageJson,
pluginsModulesPath,
Expand Down
7 changes: 5 additions & 2 deletions src/runtimes/node/bundler.js
Expand Up @@ -3,7 +3,7 @@ const { basename, dirname, extname, resolve, join } = require('path')
const esbuild = require('@netlify/esbuild')
const { tmpName } = require('tmp-promise')

const { RUNTIME_JS } = require('../../utils/consts')
const { JS_BUNDLER_ESBUILD, RUNTIME_JS } = require('../../utils/consts')
const { getPathWithExtension, safeUnlink } = require('../../utils/fs')

const { getBundlerTarget } = require('./bundler_target')
Expand Down Expand Up @@ -106,7 +106,10 @@ const bundleJsFile = async function ({
warnings,
}
} catch (error) {
error.customErrorInfo = { type: 'functionsBundling', location: { functionName: name, runtime: RUNTIME_JS } }
error.customErrorInfo = {
type: 'functionsBundling',
location: { bundler: JS_BUNDLER_ESBUILD, functionName: name, runtime: RUNTIME_JS },
}

throw error
}
Expand Down
1 change: 1 addition & 0 deletions src/runtimes/node/index.js
Expand Up @@ -70,6 +70,7 @@ const zipFunction = async function ({
featureFlags,
filename,
mainFile,
name,
pluginsModulesPath,
srcDir,
srcPath,
Expand Down
10 changes: 9 additions & 1 deletion src/runtimes/node/list_imports.js
Expand Up @@ -2,6 +2,7 @@ const esbuild = require('@netlify/esbuild')
const isBuiltinModule = require('is-builtin-module')
const { tmpName } = require('tmp-promise')

const { JS_BUNDLER_ZISI, RUNTIME_JS } = require('../../utils/consts')
const { safeUnlink } = require('../../utils/fs')

// Maximum number of log messages that an esbuild instance will produce. This
Expand All @@ -28,7 +29,7 @@ const getListImportsPlugin = ({ imports, path }) => ({
},
})

const listImports = async ({ path }) => {
const listImports = async ({ functionName, path }) => {
// We're not interested in the output that esbuild generates, we're just
// using it for its parsing capabilities in order to find import/require
// statements. However, if we don't give esbuild a path in `outfile`, it
Expand All @@ -49,6 +50,13 @@ const listImports = async ({ path }) => {
plugins: [getListImportsPlugin({ imports, path })],
target: 'esnext',
})
} catch (error) {
error.customErrorInfo = {
type: 'functionsBundling',
location: { bundler: JS_BUNDLER_ZISI, functionName, runtime: RUNTIME_JS },
}

throw error
} finally {
await safeUnlink(targetPath)
}
Expand Down
2 changes: 2 additions & 0 deletions src/runtimes/node/src_files.js
Expand Up @@ -57,6 +57,7 @@ const getSrcFilesAndExternalModules = async function ({
includedFiles = [],
includedFilesBasePath,
mainFile,
name,
pluginsModulesPath,
srcDir,
srcPath,
Expand All @@ -69,6 +70,7 @@ const getSrcFilesAndExternalModules = async function ({
featureFlags,
srcPath,
mainFile,
name,
srcDir,
stat,
pluginsModulesPath,
Expand Down
1 change: 1 addition & 0 deletions src/runtimes/node/zip_esbuild.js
Expand Up @@ -76,6 +76,7 @@ const zipEsbuild = async ({
includedFiles: [...(config.includedFiles || []), ...additionalPaths],
includedFilesBasePath: config.includedFilesBasePath || basePath,
mainFile,
name,
srcPath,
srcDir,
pluginsModulesPath,
Expand Down
2 changes: 2 additions & 0 deletions src/runtimes/node/zip_zisi.js
Expand Up @@ -15,6 +15,7 @@ const zipZisi = async ({
featureFlags,
filename,
mainFile,
name,
pluginsModulesPath,
srcDir,
srcPath,
Expand All @@ -27,6 +28,7 @@ const zipZisi = async ({
includedFiles: config.includedFiles,
includedFilesBasePath: config.includedFilesBasePath || basePath,
mainFile,
name,
pluginsModulesPath,
srcDir,
srcPath,
Expand Down
5 changes: 5 additions & 0 deletions tests/fixtures/node-syntax-error/function.js
@@ -0,0 +1,5 @@
const immutable = 1

immutable = 3

module.exports = { immutable }
41 changes: 27 additions & 14 deletions tests/main.js
Expand Up @@ -24,7 +24,12 @@ const shellUtilsStub = sinon.stub(shellUtils, 'runCommand')

const { zipFunction, listFunctions, listFunctionsFiles } = require('..')
const { ESBUILD_LOG_LIMIT } = require('../src/runtimes/node/bundler')
const { JS_BUNDLER_ESBUILD: ESBUILD, JS_BUNDLER_ESBUILD_ZISI, JS_BUNDLER_ZISI } = require('../src/utils/consts')
const {
JS_BUNDLER_ESBUILD: ESBUILD,
JS_BUNDLER_ESBUILD_ZISI,
JS_BUNDLER_ZISI,
JS_BUNDLER_ESBUILD,
} = require('../src/utils/consts')

const { getRequires, zipNode, zipFixture, unzipFiles, zipCheckFunctions, FIXTURES_DIR } = require('./helpers/main')
const { computeSha1 } = require('./helpers/sha')
Expand Down Expand Up @@ -1570,20 +1575,28 @@ test('Throws an error if the `archiveFormat` property contains an invalid value`
)
})

test('Adds `type: "functionsBundling"` to esbuild bundling errors', async (t) => {
try {
await zipNode(t, 'node-module-missing', {
opts: { config: { '*': { nodeBundler: ESBUILD } } },
})
testMany(
'Adds `type: "functionsBundling"` to user errors (esbuild)',
['bundler_default_parse_esbuild', 'bundler_esbuild'],
async (options, t) => {
const bundler = options.config['*'].nodeBundler

t.fail('Function did not throw')
} catch (error) {
t.deepEqual(error.customErrorInfo, {
type: 'functionsBundling',
location: { functionName: 'function', runtime: 'js' },
})
}
})
try {
await zipNode(t, 'node-syntax-error', {
opts: options,
})

t.fail('Bundling should have thrown')
} catch (error) {
const { customErrorInfo } = error

t.is(customErrorInfo.type, 'functionsBundling')
t.is(customErrorInfo.location.bundler, bundler === JS_BUNDLER_ESBUILD ? ESBUILD : JS_BUNDLER_ZISI)
t.is(customErrorInfo.location.functionName, 'function')
t.is(customErrorInfo.location.runtime, 'js')
}
},
)

test('Returns a list of all modules with dynamic imports in a `nodeModulesWithDynamicImports` property', async (t) => {
const fixtureName = 'node-module-dynamic-import'
Expand Down

1 comment on commit b2bf035

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

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

⏱ Benchmark results

largeDepsEsbuild: 12.8s

largeDepsZisi: 1m 7.5s

Please sign in to comment.