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

Feat(medusa): config error handling in loaders #2514

Merged
merged 9 commits into from
Nov 2, 2022

Conversation

pKorsholm
Copy link
Contributor

@pKorsholm pKorsholm commented Oct 31, 2022

What

  • add error handling when loading project config

How

  • Add error parameter to get-medusa-config result if an error was thrown (previously we returned an empty config)
  • Discussion:
    A different, but equally valid approach could be just throwing the error rather than creating an error parameter. This causes a more ugly output without warnings and changes the api a bit but it would force error handling. wdyt?

Why

  • cli would fail with database error databaseMissingDriverError if config was invalid, ex. missing a comma

example (missing , in config)

old output

Successfully compiled 2 files with Babel (143ms).
[medusa-config] ⚠️ redis_url not found. A fake redis instance will be used.
[medusa-config] ⚠️ database_type not found. fallback to default sqlite.
info:    Using flag MEDUSA_FF_ORDER_EDITING from environment with value true
info:    Using flag MEDUSA_FF_SALES_CHANNELS from environment with value true
info:    Using flag MEDUSA_FF_TAX_INCLUSIVE_PRICING from environment with value true
info:    Using fake Redis
✔ Models initialized – 13ms
✔ Plugin models initialized – 0ms
✔ Repositories initialized – 17ms
⠋ Initializing databaseMissingDriverError: Wrong driver: "undefined" given. Supported drivers are: "aurora-data-api", "aurora-data-api-pg", "better-sqlite3", "capacitor", "cockroachdb", "cordova", "expo", "mariadb", "mongodb", "mssql", "mysql", "nativescript", "oracle", "postgres", "react-native", "sap", "sqlite", "sqljs".

new output

Successfully compiled 2 files with Babel (185ms).
error:    Error in loading config: Unexpected identifier
error:    /Users/phko/projects/community/my-medusa-store/medusa-config.js:129
  plugins,
  ^^^^^^^

SyntaxError: Unexpected identifier
    at compileFunction (<anonymous>)
    at Object.compileFunction (node:vm:352:18)
    at wrapSafe (node:internal/modules/cjs/loader:1033:15)
    at Module._compile (node:internal/modules/cjs/loader:1069:27)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1159:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Module.require (node:internal/modules/cjs/loader:1005:19)
    at require (node:internal/modules/cjs/helpers:102:18)
    at getConfigFile (/Users/phko/projects/community/my-medusa-store/node_modules/medusa-core-utils/dist/get-config-file.js:26:20)

@changeset-bot
Copy link

changeset-bot bot commented Oct 31, 2022

🦋 Changeset detected

Latest commit: eb5bdd2

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
medusa-core-utils Patch
@medusajs/medusa Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pKorsholm pKorsholm marked this pull request as ready for review October 31, 2022 11:59
@pKorsholm pKorsholm requested a review from a team as a code owner October 31, 2022 11:59
Copy link
Contributor

@olivermrbl olivermrbl left a comment

Choose a reason for hiding this comment

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

Love this!

@olivermrbl
Copy link
Contributor

@srindom Are you cool with this one?

@kodiakhq kodiakhq bot merged commit ea3d738 into develop Nov 2, 2022
@kodiakhq kodiakhq bot deleted the feat/config-error-handling branch November 2, 2022 18:58
@github-actions github-actions bot mentioned this pull request Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants