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

chore(cli): Telemetry robustness and command name convention #8747

Merged
merged 65 commits into from
Jun 28, 2023

Conversation

Josh-Walker-GM
Copy link
Collaborator

@Josh-Walker-GM Josh-Walker-GM commented Jun 26, 2023

WIP

Changes

  1. Makes the OTel telemetry more robust:
    • Shutdown is now a sync function so can be directly included within the process.on('exit', ...) event. This ensures telemetry is always shutdown
    • Fixes to some incorrect file IO
    • More error handling within the telemetry sending process to prevent loops of errors
  2. Command names recorded by telemetry are all now just given directly as hard coded strings. Just adopting one way of doing it so we're consistent and clear.
    • Prevents inadvertent command names reported to telemetry which contain the yargs positional argument declarations. E.g. cell vs cell <name>

Minor changes
I'm happy to revert the following changes if they're contentious:

  1. Helper function to exit with an error lib/exit.js: exitWithError. This ensures consistent output styling and consistency in functionality such as setting the exit code.
    • This is more UX and so we'll like iterate on this.
  2. Included example use of this exitWithError within devHandler.js. Minimal usage which occurs only on port collisions.

Pushed

  1. Removing all the handler level try/catch where they are no longer needed.
    • Let's have a separate PR for that
  2. Implementing exitWithError where it would be appropriate.
    • More UX, deserves discussion and iteration, best for a separate PR

@Josh-Walker-GM Josh-Walker-GM added the release:chore This PR is a chore (means nothing for users) label Jun 26, 2023
@Josh-Walker-GM Josh-Walker-GM added the fixture-ok Override the test project fixture check label Jun 26, 2023
@jtoar jtoar merged commit 47b078a into main Jun 28, 2023
@jtoar jtoar deleted the jgmw-cli/no-process-exit branch June 28, 2023 19:24
@redwoodjs-bot redwoodjs-bot bot added this to the next-release milestone Jun 28, 2023
@jtoar jtoar modified the milestones: next-release, v6.0.0 Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixture-ok Override the test project fixture check release:chore This PR is a chore (means nothing for users)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants