Update npmauth to handle retries cleanly

[mastrzyz]

Task name: NPMAuthenticate

Description:

The NPM Authenticate task leverages live services and can be very flaky for us , so we used the retryOnTaskFailure setting in the main task to try and mitigate this.

We encounter the following error when we have a retry :

Adding authentication to the .npmrc file at D:\a_work\3\s*********.npmrc
##[debug]SAVE_NPMRC_PATH=D:\a_work\3\npmAuthenticate\FyuhLH
##[debug]SAVE_NPMRC_PATH=D:\a_work\3\npmAuthenticate\FyuhLH
##[debug]NPM_AUTHENTICATE_TEMP_DIRECTORY=D:\a_work\3\npmAuthenticate
##[debug]NPM_AUTHENTICATE_TEMP_DIRECTORY=D:\a_work\3\npmAuthenticate
##[debug]rm -rf D:\a_work\3\npmAuthenticate
##[debug]task result: Failed
##[error]Error: ENOENT: no such file or directory, open 'D:\a_work\3\npmAuthenticate\FyuhLH\index.json'
##[debug]Processed: ##vso[task.issue type=error;]Error: ENOENT: no such file or directory, open 'D:\a_work\3\npmAuthenticate\FyuhLH\index.json'
##[debug]Processed: ##vso[task.complete result=Failed;]Error: ENOENT: no such file or directory, open 'D:\a_work\3\npmAuthenticate\FyuhLH\index.json'
##[warning]RetryHelper encountered task failure, will retry (attempt #: 2 out of 4) after 4000 ms
This looks to be due to the fact that after the first attempt to authenticate we timed out and the following catch code is run :

main().catch(error => {
if(tl.getVariable("NPM_AUTHENTICATE_TEMP_DIRECTORY")) {
tl.rmRF(tl.getVariable("NPM_AUTHENTICATE_TEMP_DIRECTORY"));
}
tl.setResult(tl.TaskResult.Failed, error);
});
This wipes the directory NPM_AUTHENTICATE_TEMP_DIRECTORY but not the variables.

thus on retry the following code gets executed :

if (tl.getVariable("SAVE_NPMRC_PATH")) {
     saveNpmrcPath = tl.getVariable("SAVE_NPMRC_PATH");
}
else {
    let tempPath = tl.getVariable('Agent.BuildDirectory') || tl.getVariable('Agent.TempDirectory');
    tempPath = path.join(tempPath, 'npmAuthenticate');
    tl.mkdirP(tempPath);
    saveNpmrcPath = fs.mkdtempSync(tempPath + path.sep); 
    tl.setVariable("SAVE_NPMRC_PATH", saveNpmrcPath, false);
    tl.setVariable("NPM_AUTHENTICATE_TEMP_DIRECTORY", tempPath, false);
}

And since SAVE_NPMRC_PATH was previously set, the code think the directory should be there, but it is not.

Simple solution would be to wipe this variable alongside wiping the root directory.

main().catch(error => {
if(tl.getVariable("NPM_AUTHENTICATE_TEMP_DIRECTORY")) {
tl.rmRF(tl.getVariable("NPM_AUTHENTICATE_TEMP_DIRECTORY"));
// Clear the variales after we rm-rf the main root directory
tl.setVariable("SAVE_NPMRC_PATH", "", false);
tl.setVariable("NPM_AUTHENTICATE_TEMP_DIRECTORY", "", false);
}
tl.setResult(tl.TaskResult.Failed, error);
});

Does that seem good? am I missing something important here.

Documentation changes required: (Y/N) N

Added unit tests: (Y/N) -> Current task has no UT's // TODO - add real tests

Attached related issue: (Y/N) #15913

Checklist:

• Task version was bumped - please check instruction how to do it
• Checked that applied changes work as expected -> Would appreciate some guidance here, how would I try this out in our repository?

I tried the following code to test out my changes :

Commented out my 'fix' and called main immediately after the failure to fake a retry attempt

main().catch(error => {
    if (tl.getVariable("NPM_AUTHENTICATE_TEMP_DIRECTORY")) {
        tl.rmRF(tl.getVariable("NPM_AUTHENTICATE_TEMP_DIRECTORY"));
        // Clear the variales after we rm-rf the main root directory
      //  tl.setVariable("SAVE_NPMRC_PATH", "", false);
      //  tl.setVariable("NPM_AUTHENTICATE_TEMP_DIRECTORY", "", false);
    }
    main()
    tl.setResult(tl.TaskResult.Failed, error);
});

Result ->

Uncommented my 'fix'

main().catch(error => {
if (tl.getVariable("NPM_AUTHENTICATE_TEMP_DIRECTORY")) {
tl.rmRF(tl.getVariable("NPM_AUTHENTICATE_TEMP_DIRECTORY"));
// Clear the variales after we rm-rf the main root directory
tl.setVariable("SAVE_NPMRC_PATH", "", false);
tl.setVariable("NPM_AUTHENTICATE_TEMP_DIRECTORY", "", false);
}
main()
tl.setResult(tl.TaskResult.Failed, error);
});

No more error!

[mastrzyz]

@aasim provided an update with how I tested this, let me know if it seems good.

[zhenghao104]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 4 pipeline(s).