Replies: 6 comments
-
@nferch This is where we're handling the Can you help me better understand what you'd like to see changed? Is it that you want the option to always throw on that error? If so, we could make that configurable either through the |
Beta Was this translation helpful? Give feedback.
-
@mdonnalley yeah, I came across that when debugging and thought that might be the place. Out of curiosity, under what condition is it useful to swallow that error? Is this the same codepath when a command doesn't actually exist? I've been seeing the behavior when there's some bad code introduced or some other build error. What has been happening for us is that all commands fail to import so it appears that our CLI has no commands at all. Which is a confusing failure mode for both us and our users, as well as a bit awkward to detect in smoketests. So I'd prefer to see it handled as a fatal error rather than a command not existing. Having an option to throw the error would probably be fine, but I also wonder if there'd be some way to distinguish the error being thrown on import rather than the import missing altogether. Does that make sense? |
Beta Was this translation helpful? Give feedback.
-
Off the top of my head, I think the idea was that you don't want one bad actor to bring down the entire CLI. So imagine a scenario where someone has installed a plugin that has a faulty
That code path is here.
Are you able to share a repo that replicates this? Thanks! |
Beta Was this translation helpful? Give feedback.
-
That makes sense. In our case these are all commands in our own CLI, so we'd prefer for the CLI to fail fatally.
The CLI in question is located at https://github.com/dxos/dxos/tree/main/packages/devtools/cli . It's currently in a non-broken state, but you can simulate the failure by adding an invalid import to any of the commands. |
Beta Was this translation helpful? Give feedback.
-
@nferch Sounds like we might just want to make your desired behavior configurable. Is this something you only need for your tests and build processes? If that's the case, then I'd say that it should be an environment variable. But if it's something that you want in production, then it should be a setting in the package.json We don't have the bandwidth to prioritize this at the moment so I'd recommend making the PR yourself if it's something you need soon. If you do decide to do that, I'm happy to help with any questions you might have. |
Beta Was this translation helpful? Give feedback.
-
@mdonnalley Thanks for the help! This isn't a super-high priority for us either, but it's good to have this issue for reference for when it inevitably comes up again. |
Beta Was this translation helpful? Give feedback.
-
Is your feature request related to a problem? Please describe.
Build and programming errors can result in broken dependencies that don't produce errors until runtime.
Describe the solution you'd like
An error in loading a command, such as
MODULE_NOT_FOUND
should cause the CLI to fail rather than producing a confusing 'Invalid command` error.Describe alternatives you've considered
I'm not able to find a place in the oclif API to catch command load errors in order to display a more explicit error message
Beta Was this translation helpful? Give feedback.
All reactions