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

Throw an error if serverPlugin is not defined #633

Closed
2 tasks done
mbelsky opened this issue Jun 19, 2023 · 3 comments · Fixed by #649
Closed
2 tasks done

Throw an error if serverPlugin is not defined #633

mbelsky opened this issue Jun 19, 2023 · 3 comments · Fixed by #649
Labels
feature request New feature to be added good first issue Good for newcomers

Comments

@mbelsky
Copy link

mbelsky commented Jun 19, 2023

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the feature has not already been requested

🚀 Feature Proposal

Throw a well described exception if serverPlugin is not defined (node_modules/fastify-cli/util.js, requireServerPluginFromPath)

Motivation

Hi team,

I had followed Getting Started docs page, copied the code from 'You First Server' section and then used command to run a server from 'Run Your Server From CLI' section. The execution failed with the following exception:

TypeError: Cannot read properties of undefined (reading 'length')
    at isInvalidAsyncPlugin (/Users/mbelsky/gh/xox-mm/node_modules/fastify-cli/util.js:45:17)
    at requireServerPluginFromPath (/Users/mbelsky/gh/xox-mm/node_modules/fastify-cli/util.js:84:7)
    at async runFastify (/Users/mbelsky/gh/xox-mm/node_modules/fastify-cli/start.js:90:12)

Then it took me some time to realize the start file must have an exported plugin. "fastify-cli" package version: 5.7.1

I believe adding a well described exception throw can help other users to spot the issue faster. If you find the proposal useful I'd like to implement it.

Example

if (!serverPlugin) {
  throw new Error(`${resolvedModulePath} does not export a plugin function`)
}
@mcollina
Copy link
Member

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

@mbelsky
Copy link
Author

mbelsky commented Jun 20, 2023

Yes, I'd like to address the issue and cover it with ut.

@mcollina mcollina added bug Confirmed bug good first issue Good for newcomers feature request New feature to be added and removed bug Confirmed bug labels Jun 20, 2023
@GHNewbiee
Copy link

I do face the same incident.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature to be added good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants