Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve error handling when the functions directory does not exist #19

Open
ehmicky opened this issue Nov 5, 2020 · 4 comments
Open

Comments

@ehmicky
Copy link
Collaborator

ehmicky commented Nov 5, 2020

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).

@bencao
Copy link
Owner

bencao commented Nov 21, 2020

closed with b42791a

@bencao bencao closed this as completed Nov 21, 2020
@ehmicky
Copy link
Collaborator Author

ehmicky commented Nov 21, 2020

Hi @bencao,

Thanks! 🎉

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()

@bencao
Copy link
Owner

bencao commented Nov 23, 2020

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 bencao reopened this Nov 23, 2020
@ehmicky
Copy link
Collaborator Author

ehmicky commented Nov 23, 2020

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants