netlify-plugin-inline-functions-env icon indicating copy to clipboard operation
netlify-plugin-inline-functions-env copied to clipboard

Improve error handling when the functions directory does not exist

Open ehmicky opened this issue 4 years ago • 4 comments

When the user has not specified any Functions directory, this plugin currently fails with:

No function directory was specified 

or

Could not list Netlify Functions files
Functions folder does not exist: dist/functions 

However, this is reported as a plugin error. It would be better to report it as a user error using utils.build.failBuild() by checking at the beginning whether constants.FUNCTIONS_SRC points to an existing directory (using for example path-exists).

ehmicky avatar Nov 05 '20 21:11 ehmicky

closed with https://github.com/bencao/netlify-plugin-inline-functions-env/commit/b42791a070d97afe178447390f5cc4d44e22208e

bencao avatar Nov 21 '20 07:11 bencao

Hi @bencao,

Thanks! :tada:

One note though: utils.functions.listAll() can throw for other reasons than the functions folder not existing:

https://github.com/bencao/netlify-plugin-inline-functions-env/blob/master/index.js#L35-L41

https://github.com/netlify/build/blob/master/packages/functions-utils/src/main.js#L60-L70

Notably, utils.functions.listAll() throws if a Function file is requiring a dependency which is missing from the site (which is a very common error). In that case, the current code would show Failed to inline function files because netlify function folder was not configured or pointed to a wrong folder, please check your configuration which would be confusing considering the functions folder would most likely exist.

Instead, explicitly checking whether constants.FUNCTIONS_SRC exists, while still allowing any error from listAll() to propagate would give the correct behavior:

if (constants.FUNCTIONS_SRC === undefined || !(await pathExists(constants.FUNCTIONS_SRC))) {
  return utils.build.failBuild('Failed to inline functions files because Netlify functions folder was not configured or pointed to a wrong folder, please check your configuration')
}

const netlifyFunctions = await utils.functions.listAll()

ehmicky avatar Nov 21 '20 14:11 ehmicky

hi @ehmicky, what if in the plugin we catch the exception and throw with whatever message returned by utils.functions.listAll()? Checking explicitly for undefined functions src folder feels like a bit burden to a plugin? (assume the above logic has to be implemented again and again by different plugins)

bencao avatar Nov 23 '20 18:11 bencao

Good point! Actually, I'll even say plugins should not have to add any error handling since this utility is provided by Netlify. Checking the code, it looks like those errors are actually already handled as user errors:

https://github.com/netlify/build/blob/master/packages/functions-utils/src/main.js#L60-L70

https://github.com/netlify/build/blob/8779420d714e1b916c8a2cedf1e2593016f466f6/packages/build/src/plugins/child/utils.js#L54

Based on this, I am wondering how I got the following behavior:

However, this is reported as a plugin error.

If you remove any error handling on this function, then try to build with a functions folder, could you please confirm whether this is reported as a user error or as a plugin error? Thanks!

ehmicky avatar Nov 23 '20 20:11 ehmicky