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

fix: cache stdin #935

Merged
merged 7 commits into from
Feb 6, 2024
Merged

fix: cache stdin #935

merged 7 commits into from
Feb 6, 2024

Conversation

mdonnalley
Copy link
Contributor

@mdonnalley mdonnalley commented Feb 2, 2024

Currently, flags and args read from stdin are lost whenever Parser.parse is called multiple times. This PR caches the value read from stdin so that Parser.parse can be called multiple times.

This PR also:

  • decreases timeout for reading stdin (from 100ms to 10ms)
  • Flags with allowStdin: 'only' are no longer required to have - provided as the value (it's assumed)

@W-14954770@
forcedotcom/cli#2690

cristiand391
cristiand391 previously approved these changes Feb 5, 2024
Copy link
Member

@cristiand391 cristiand391 left a comment

Choose a reason for hiding this comment

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

approved with one question about globals.

cristiand391
cristiand391 previously approved these changes Feb 5, 2024
@cristiand391
Copy link
Member

cristiand391 commented Feb 6, 2024

QA notes:

setup:
@salesforce/cli/2.27.6 darwin-x64 node-v20.10.0

linked this branch of oclif/core into plugin-auth and plugin-telemetry

authUrl:

force://PlatformCLI::5Aep861LaOi0eFB3lg1.HqHPmKgMQWVyfQak6yA5VoPOnkSv2T2Flr23_ADd_06_9V3dX7_lbE9bgNTguJB1dDA@enterprise-innovation-7434-dev-ed.scratch.my.salesforce.com

🟢 stdin is cached in global.oclif on first parse

telemetry enabled (with debug mode)

ignore the Unable to get oclif performance metrics Error, that always happens when plugin-telemetry is linked, doesn't happen on prod.

➜  plugin-auth git:(main) ✗ echo $(pbpaste) | DEBUG=*sf:telemetry* SF_TELEMETRY_DEBUG=true SF_DISABLE_TELEMETRY=false sf org login sfdx-url --sfdx-url-stdin -
 ›   Warning: @salesforce/plugin-auth is a linked ESM module and cannot be auto-transpiled. Existing compiled source will be used instead.
 ›   Warning: @salesforce/plugin-telemetry is a linked ESM module and cannot be auto-transpiled. Existing compiled source will be used instead.
  sf:telemetry Usage acknowledgement file already exists +0ms
  sf:telemetry Using telemetry logging file /var/folders/0m/49576tls6n3fxd4vqvn3hzv80000gq/T/sf-telemetry/telemetry-6714d31c27f74a1872d91fc3b22a57b90f232462.log +1ms
  sf:telemetry Setting up process exit handler +5ms
Error (1): Error authenticating with the refresh token due to: expired access/refresh token

  sf:telemetry Unable to get oclif performance metrics Error: Perf results not available. Did you forget to call await Performance.collect()?
    at get oclifPerf [as oclifPerf] (/Users/cdominguez/code/gh/oclif/core/lib/performance.js:217:15)
    at process.<anonymous> (file:///Users/cdominguez/code/gh/sf/plugin-telemetry/lib/hooks/telemetryPrerun.js:71:41)
    at Object.onceWrapper (node:events:629:26)
    at process.emit (node:events:526:35)
    at process.processEmit (/Users/cdominguez/code/gh/sf/plugin-auth/node_modules/signal-exit/index.js:191:37)
    at process.processEmit (/Users/cdominguez/code/gh/sf/plugin-telemetry/node_modules/signal-exit/index.js:191:37)
    at process.processEmit [as emit] (/Users/cdominguez/.nvm/versions/node/v20.10.0/lib/node_modules/@salesforce/cli/node_modules/signal-exit/index.js:191:37)
    at process.exit (node:internal/process/per_thread:193:15)
    at Object.exit (/Users/cdominguez/.nvm/versions/node/v20.10.0/lib/node_modules/@salesforce/cli/node_modules/@oclif/core/lib/errors/handle.js:20:17)
    at Module.handle (/Users/cdominguez/.nvm/versions/node/v20.10.0/lib/node_modules/@salesforce/cli/node_modules/@oclif/core/lib/errors/handle.js:46:26) +3s
  sf:telemetry DEBUG MODE. Run the uploader manually with the following command:
  sf:telemetry /Users/cdominguez/code/gh/sf/plugin-telemetry/processes/upload.js /Users/cdominguez/Library/Caches/sf /var/folders/0m/49576tls6n3fxd4vqvn3hzv80000gq/T/sf-t
elemetry/telemetry-6714d31c27f74a1872d91fc3b22a57b90f232462.log +3ms

telemetry disabled (single parser call, was working before):

➜  plugin-auth git:(main) ✗ echo $(pbpaste) | SF_DISABLE_TELEMETRY=true sf org login sfdx-url --sfdx-url-stdin -
 ›   Warning: @salesforce/plugin-auth is a linked ESM module and cannot be auto-transpiled. Existing compiled source will be used instead.
 ›   Warning: @salesforce/plugin-telemetry is a linked ESM module and cannot be auto-transpiled. Existing compiled source will be used instead.
Error (1): Error authenticating with the refresh token due to: expired access/refresh token

UPDATE: fixed
debug logs show a parser call got nothing resolved from stdin:

➜  plugin-auth git:(main) ✗ echo $(pbpaste) | DEBUG='*../parser*' CLI_FLAGS_DEBUG=1 SF_TELEMETRY_DEBUG=true SF_DISABLE_TELEMETRY=false sf org login sfdx-url --sfdx-url-st
din -
 ›   Warning: @salesforce/plugin-auth is a linked ESM module and cannot be auto-transpiled. Existing compiled source will be used instead.
 ›   Warning: @salesforce/plugin-telemetry is a linked ESM module and cannot be auto-transpiled. Existing compiled source will be used instead.
  ../parser input: --sfdx-url-stdin - +0ms
  ../parser available flags: --sfdx-url-file --sfdx-url-stdin --set-default-dev-hub --set-default --alias --no-prompt --loglevel --json +0ms
  ../parser resolved from stdin force://PlatformCLI::5Aep861LaOi0eFB3lg1.HqHPmKgMQWVyfQak6yA5VoPOnkSv2T2Flr23_ADd_06_9V3dX7_lbE9bgNTguJB1dDA@enterprise-innovation-7434-de
v-ed.scratch.my.salesforce.com +3ms
  ../parser input: --sfdx-url-stdin - +31ms
  ../parser available flags: --json --sfdx-url-file --sfdx-url-stdin --set-default-dev-hub --set-default --alias --no-prompt --loglevel +0ms
  ../parser stdin aborted +54ms
  ../parser resolved from stdin  +0ms
Error (1): Error authenticating with the refresh token due to: expired access/refresh token

both telemetry and plugin-auth were able to get the value, couldn't track the call where it resolves to nothing.

🟢 allowStdin: 'only' flags work with and without - value

@cristiand391 cristiand391 merged commit c8bf886 into main Feb 6, 2024
59 checks passed
@cristiand391 cristiand391 deleted the mdonnalley/multiple-parses branch February 6, 2024 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants