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: return runtime versions used by the application with a doctor hook #1763

Merged
merged 4 commits into from
Mar 26, 2024

Conversation

zimeg
Copy link
Member

@zimeg zimeg commented Mar 26, 2024

Summary

This PR introduces a doctor hook that returns runtime versions for the application's execution environment in a structured way. The doctor() demonstrate the expected response shape!

The node version is included in this response, as well as the v8 and modules version. TIL the modules version can be a pain point when installing dependencies so thought it was worth including here.

Open to any adjustments to output. These versions were taken from `process.versions`
> process.versions
{
  node: '20.11.1',
  acorn: '8.11.2',
  ada: '2.7.4',
  ares: '1.20.1',
  base64: '0.5.1',
  brotli: '1.0.9',
  cjs_module_lexer: '1.2.2',
  cldr: '43.1',
  icu: '73.2',
  llhttp: '8.1.1',
  modules: '115',
  napi: '9',
  nghttp2: '1.58.0',
  openssl: '3.0.13',
  simdutf: '4.0.4',
  tz: '2023c',
  undici: '5.28.3',
  unicode: '15.0',
  uv: '1.48.0',
  uvwasi: '0.0.19',
  v8: '11.3.244.8-node.17',
  zlib: '1.2.13.1-motley-5daffc7'
}

Preview

node

Reviewers

Try this on your very own app with a special build of the CLI:

$ slack create nod -e bolt
$ cd nod
$ npm install ~/path/to/node-slack-sdk/packages/cli-hooks --save-dev
$ slack doctor

Requirements

@zimeg zimeg added semver:minor enhancement M-T: A feature request for new functionality pkg:cli-hooks applies to `@slack/cli-hooks` labels Mar 26, 2024
@zimeg zimeg added this to the [email protected] milestone Mar 26, 2024
@zimeg zimeg requested review from filmaj and WilliamBergamin March 26, 2024 19:10
@zimeg zimeg self-assigned this Mar 26, 2024
@filmaj
Copy link
Contributor

filmaj commented Mar 26, 2024

@zimeg Hmm I tried creating a project w/ the feat-doctor-hook branch of the CLI, but I get a cryptic error trying to create a project with your instructions (and selecting 'node.js bolt for js' from the starter template prompt):

➜ slak create dr-hook-node -e bolt
? Select a starter template to build from: Node.js Bolt for JavaScript
⚙️  Creating a new Slack app in ~/src/dr-hook-node

⚠️   Error installing project dependencies: exit status 1


Check /Users/fmaj/.slack/logs/slack-debug-20240326.log for full error logs

🚫 exit status 1

The debug log doesn't say much more than just "exit status 1" 🤷 any ideas?

@zimeg
Copy link
Member Author

zimeg commented Mar 26, 2024

@filmaj uh oh... I don't that's related to the changes of that CLI branch but I've seen this before and forget why 🤔

From what I remember this is an error caused by this command:

$ npm install --no-package-lock --no-audit --prefer-offline --progress=false --loglevel=verbose .

I see something similar but different when I don't have Node installed. The project code is usually cloned alright but I'll have to cd <project> && npm install myself.

The only guess I have is a local @slack/cli-hooks was linked and is being used during the installation, but I wouldn't think that'd error 🤔

@zimeg
Copy link
Member Author

zimeg commented Mar 26, 2024

The debug logs don't seem to have much info for this either... Probably worth an improvement sometime soon

@filmaj
Copy link
Contributor

filmaj commented Mar 26, 2024

Interesting, ok, thanks for the info. So the CLI actually created the app dir, and it seems to be populated. I can npm i in it just fine. If I try to run the command you posted in that dir, however, I see:

npm verb type range
npm verb stack @slack/cli-hooks: No matching version found for @slack/cli-hooks@^1.0.0.
npm verb stack     at module.exports (/Users/fmaj/.nvm/versions/node/v18.15.0/lib/node_modules/npm/node_modules/npm-pick-manifest/lib/index.js:209:23)
npm verb stack     at RegistryFetcher.manifest (/Users/fmaj/.nvm/versions/node/v18.15.0/lib/node_modules/npm/node_modules/pacote/lib/registry.js:119:22)
npm verb stack     at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
npm verb stack     at async [nodeFromEdge] (/Users/fmaj/.nvm/versions/node/v18.15.0/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/arborist/build-ideal-tree.js:1059:19)
npm verb stack     at async [buildDepStep] (/Users/fmaj/.nvm/versions/node/v18.15.0/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/arborist/build-ideal-tree.js:929:11)
npm verb stack     at async Arborist.buildIdealTree (/Users/fmaj/.nvm/versions/node/v18.15.0/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/arborist/build-ideal-tree.js:207:7)
npm verb stack     at async Promise.all (index 1)
npm verb stack     at async Arborist.reify (/Users/fmaj/.nvm/versions/node/v18.15.0/lib/node_modules/npm/node_modules/@npmcli/arborist/lib/arborist/reify.js:159:5)
npm verb stack     at async Install.exec (/Users/fmaj/.nvm/versions/node/v18.15.0/lib/node_modules/npm/lib/commands/install.js:146:5)
npm verb stack     at async module.exports (/Users/fmaj/.nvm/versions/node/v18.15.0/lib/node_modules/npm/lib/cli.js:134:5)
npm verb cwd /Users/fmaj/src/dr-hook-node
npm verb Darwin 23.4.0
npm verb node v18.15.0
npm verb npm  v9.5.0
npm ERR! code ETARGET
npm ERR! notarget No matching version found for @slack/cli-hooks@^1.0.0.
npm ERR! notarget In most cases you or one of your dependencies are requesting
npm ERR! notarget a package version that doesn't exist.

Bizarre, because npm i was able to handle it just fine:

➜ grep version node_modules/@slack/cli-hooks/package.json
  "version": "1.0.0",```

@zimeg
Copy link
Member Author

zimeg commented Mar 26, 2024

📝 For the future searcher stumbling upon this, the following command fixes the "No matching version found" error:

$ npm cache clean --force

Copy link
Contributor

@filmaj filmaj left a comment

Choose a reason for hiding this comment

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

Yass works great on node 18 and node 20.

Might need to add some more resiliency, or at least better error reporting, during the npm i invocation upon creation of an app that uses these hooks. Follow-up PR? I'm not sure that I would ever be able to figure out something was wrong with installing npm dependencies without your hand holding.


/**
* Standardized communication format between the SDK and CLI regarding runtimes.
* @typedef DoctorResponse
Copy link
Contributor

Choose a reason for hiding this comment

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

look Ma, no build step!
❤️

Copy link
Member Author

Choose a reason for hiding this comment

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

💪 🤧

@zimeg zimeg added the release label Mar 26, 2024
@zimeg
Copy link
Member Author

zimeg commented Mar 26, 2024

@filmaj thanks for enduring the trials of testing this! I agree that much better errors can be reported for erroring installations, but will save this for a follow up. I'm not sure that it's possible to do this via hooks either (at least for the initial install) because the hooks wouldn't be installed yet! 🐣

Going tag this as a release after merging! Appreciate the quick review! 🏷️

@zimeg zimeg merged commit 3e29641 into main Mar 26, 2024
17 checks passed
@zimeg zimeg deleted the feat-doctor-hook branch March 26, 2024 23:15
@zimeg
Copy link
Member Author

zimeg commented Mar 26, 2024

Urgh as soon as I hit merge I noticed the PR title isn't prefixed with cli-hooks urgh urgh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement M-T: A feature request for new functionality pkg:cli-hooks applies to `@slack/cli-hooks` release semver:minor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants