core icon indicating copy to clipboard operation
core copied to clipboard

Hooks are not working properly

Open alessandrotesoro opened this issue 3 years ago • 1 comments

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

  1. When using the init hook, you can't exit the CLI by using this.error or this.exit none of those work.
  2. Because of the above, I thought of using the prerun hook, this hook doesn't fire at all.
  3. Because prerun did not work, I tried to use a custom hook, custom hooks aren't working either.

Custom hook code:

import {Hook} from '@oclif/core'

const myhook: Hook<'analytics'> = async function (opts) {
  process.stdout.write(`example custom hook here`)
}

export default myhook

Custom command run code:

public async run(): Promise<void> {
		const { args, flags } = await this.parse(Tag)

		await this.config.runHook('analytics', {id: 'my_command'})
}

package.json

"oclif": {
    "bin": "barn2-deployer",
    "dirname": "barn2-deployer",
    "commands": "./dist/commands",
    "plugins": [
      "@oclif/plugin-help",
      "@oclif/plugin-plugins"
    ],
    "topicSeparator": " ",
    "topics": {
      "hello": {
        "description": "Say hello to the world and others"
      }
    },
    "hooks": {
      "init": "./dist/hooks/init/verifyPlugin",
      "analytics": "./dist/hooks/analytics/myhook"
    }
  },

What is the expected behavior?

Core version: 1.16.4 MacOS Monterey 12.1

alessandrotesoro avatar Oct 06 '22 16:10 alessandrotesoro

@alessandrotesoro Thanks for filing an issue. oclif/core captures and returns hook failures, meaning that hooks cannot exit the process (you can see the code here). This is different from the now deprecated oclif/config which allowed hooks exit the process by throwing an error.

We made this decision primarily because the Salesforce CLI depends heavily on custom hooks and we didn't want to allow one failed hook to exit the entire process.

We're open, however, to support some sort of toggle that will allow hooks to exit if there's a strong enough demand for it. We'd also be willing to review and merge a PR if you would like to take that work up yourself.

In the meantime, I'm going to transfer this issue to the oclif/core repo since that is where any change would be implemented.

mdonnalley avatar Oct 06 '22 17:10 mdonnalley

Hey @shrutiburman, as Mike mentioned, oclif/core captures the hook failure.

If you want to make the CLI exit if the hook failed you should be able to do that where the hook is fired, see this PR for an example: https://github.com/salesforcecli/sfdx-cli/pull/677/files

you can make the CLI throw an err for all failures or filter by plugins.

cristiand391 avatar Dec 05 '22 14:12 cristiand391

I'll check it out, thanks!

shrutiburman avatar Dec 05 '22 14:12 shrutiburman

This issue has been linked to a new work item: W-12514447

git2gus[bot] avatar Feb 08 '23 18:02 git2gus[bot]