Fix npm access error(#665) #666

DerTimonius · 2022-11-15T21:39:28Z

Thank you for considering this PR

This is my first contribution to open source, so if this PR description is a bit too much, please let me know 😃

This PR fixes the npm access error that occurs after updating npm to version 9.0.0.

✖ Command failed with exit code 1: npm access ls-collaborators @upleveled/eslint-config-upleveled
npm ERR! code EUSAGE
npm ERR!
npm ERR! ls-collaborators is not a valid access command
npm ERR!
npm ERR! Set access level on published packages
npm ERR!
npm ERR! Usage:
npm ERR! npm access list packages [<user>|<scope>|<scope:team> [<package>]
npm ERR! npm access list collaborators [<package> [<user>]]
npm ERR! npm access get status [<package>]
npm ERR! npm access set status=public|private [<package>]
npm ERR! npm access set mfa=none|publish|automation [<package>]
npm ERR! npm access grant <read-only|read-write> <scope:team> [<package>]
npm ERR! npm access revoke <scope:team> [<package>]
npm ERR!
npm ERR! Options:
npm ERR! [--json] [--otp <otp>] [--registry <registry>]
npm ERR!
npm ERR! Run "npm help access" for more info

According to the npm documentation the npm access subcommands have been changed from ls-collaborators to list collaborators.

Switching

const args = ['access', 'ls-collaborators', packageName];

to

const args = ['access', 'list collaborators', packageName];

did not work, it keeps failing with the same errors.
Changing the execa arguments (on line 77) from npm to npm access list collaborators while stripping the args variable eliminated the error.

Fixes #665

source/npm/util.js

sindresorhus · 2022-11-16T09:19:21Z

const args = ['access', 'list collaborators', packageName];

You have a typo. Should be:

const args = ['access', 'list', 'collaborators', packageName];

sindresorhus · 2022-11-16T09:22:06Z

Make sure that npm test passes locally and to review your own diff

DerTimonius · 2022-11-16T16:08:04Z

Thank you for your feedback!
I had troubles running the tests on my (windows) machine yesterday, found a workaround but that clearly did not work.
I changed the line as you suggested, I also changed two tests of the prerequisites-tasks.js because it used the old npm access subcommands (not committed yet).
But running the tests (with the help of cross-env) now, I'm getting the following error with this index » version is invalid and index » version is pre-release:

TimeoutError {
     context: {},
     message: 'Connection to npm registry timed out',
 }

DerTimonius · 2022-11-16T18:41:33Z

So, I now committed the changes to the test files as well and now the Github Actions tests run without error on my forked repo 👍

ar1a · 2022-11-16T23:41:06Z

source/npm/util.js

@@ -68,7 +68,7 @@ exports.collaborators = async pkg => {
 	const packageName = pkg.name;
 	ow(packageName, ow.string);

-	const args = ['access', 'ls-collaborators', packageName];
+	const args = ['access', 'list', 'collaborators', packageName];


this needs a --json as well

schuchard · 2022-11-18T21:20:09Z

Hey @DerTimonius, looks like you're pretty close. Anything I can do to help?

DerTimonius · 2022-11-18T21:25:41Z

Hey @schuchard , thank you for your offer!
I did not have time in the last two days to take a look into this again, and now I added the --json flag. I'm not sure if I missed anything else though, so if I did, any help would be appreciated 😃

schuchard · 2022-11-21T22:05:06Z

Hi @sindresorhus, is there anything else needed to merge this PR? Thanks.

DerTimonius · 2022-11-22T08:18:13Z

I don't know why the tests are failing now with an error message that should be solved (npm access list not a valid argument).

karlhorky · 2022-11-22T10:59:24Z

Maybe it's failing on an older version of npm? (eg. one that does not yet have npm access list ... commands)

Maybe it should be conditional on what version of npm is being run.

schuchard · 2022-11-22T15:31:02Z

Seems likely. NPM 9 brought a breaking change to npm access:

npm access subcommands have been renamed

If this package intends to support npm < 9, conditional logic and tests are needed.

DerTimonius · 2022-11-23T18:11:14Z

I'm making progress: I implemented a npm version check, so that the correct subcommands are used.

Currently I have one failing test left:

prerequisite-tasks › should fail when user is not authenticated at external registry

142:   await t.throwsAsync(run(testedModule('2.0.0', {name: 'test', version:…
   143:     {message: 'You do not have write permissions required to publish th…

  Promise resolved with:

  undefined

Locally, I always get a Timeout Error on this test though...

DerTimonius · 2022-11-27T20:11:37Z

Any hints on what I'm missing?

karlhorky · 2022-12-05T15:51:04Z

So this is the error message that you're getting:

  1 test failed

  prerequisite-tasks › should fail when user is not authenticated at external registry

  /home/runner/work/np/np/test/prerequisite-tasks.js:145

   144:   process.env.NODE_ENV = 'P';                                           
   145:   await t.throwsAsync(run(testedModule('2.0.0', {name: 'test', version:…
   146:     {message: 'You do not have write permissions required to publish th…

  Promise rejected with unexpected exception:

  Error {
    context: {},
    message: 'Git tag `v2.0.0` already exists.',
  }

  Expected message to equal:

  'You do not have write permissions required to publish this package.'

  Object.exports.verifyTagDoesNotExistOnRemote (source/git-util.js:206:9)
  Task.task (source/prerequisite-tasks.js:88:5)

This indicates that the collaborators JSON here actually passed the test:

np/source/prerequisite-tasks.js

Lines 44 to 48 in 95622c0

    
           const json = JSON.parse(collaborators); 
        
           const permissions = json[username]; 
        
           if (!permissions || !permissions.includes('write')) { 
        
           	throw new Error('You do not have write permissions required to publish this package.'); 
        
           }

You could try logging out this value to see if this is truly the case.

Or, second idea: maybe the check for list collaborators is not being run at all... 🤔 Seems like you have a weird pattern here:

np/test/prerequisite-tasks.js

Lines 11 to 14 in 6d4f036

    
           const testArgs = async () => { 
        
           	const npmVersion = await versionNpm(); 
        
           	return semver.satisfies(npmVersion, '>=9.0.0') ? ['list collaborators', ' --json'] : ['ls-collaborators', '']; 
        
           };

To me testArgs should be an array, not a function...

karlhorky · 2022-12-05T15:53:07Z

Last tip, the order of your arguments in these three places is different:

np/source/npm/util.js

Line 73 in 6d4f036

    
           const args = semver.satisfies(npmVersion, '>=9.0.0') ? ['access', 'list', 'collaborators', '--json', packageName] : ['access', 'ls-collaborators', packageName];

np/test/prerequisite-tasks.js

Line 119 in 6d4f036

command: `npm access ${testArgs[0]} test${testArgs[1]}`,

np/test/prerequisite-tasks.js

Line 139 in 6d4f036

    
           command: `npm access ${testArgs[0]} test${testArgs[1]} --registry http://my.io`,

DerTimonius · 2022-12-06T17:29:13Z

Thank you for your inputs @karlhorky !

After logging out a few things, I narrowed the problem down to an issue with the external registry:

The collaborators functions return false and the task does not get to the permissions variable.
Logging the args inside the collaborators function returns a valid array.
But since it returned false, the only place where this could happen is here in source/npm/util.js, lines 78-88:

try {
		const {stdout} = await execa('npm', args);
		return stdout;
	} catch (error) {
		// Ignore non-existing package error
		if (error.stderr.includes('code E404')) {
			return false;
		}

		throw error;
	}

So, logging out the error here gives me this message:

'npm ERR! code E404\n' +
    'npm ERR! 404 Not Found - GET http://my.io/-/package/test/collaborators\n' +
    '\n' +
    'npm ERR! A complete log of this run can be found in:\n'

But the command is this:

npm access list collaborators --json test --registry http://my.io

I don't know where the -/package/test/collaborators is coming from, it's not added in the code... (or if this is even the issue).

Regarding this:

Or, second idea: maybe the check for list collaborators is not being run at all... 🤔 Seems like you have a weird pattern here:

np/test/prerequisite-tasks.js

Lines 11 to 14 in 6d4f036

const testArgs = async () => {

const npmVersion = await versionNpm();

return semver.satisfies(npmVersion, '>=9.0.0') ? ['list collaborators', ' --json'] : ['ls-collaborators', ''];

};

To me testArgs should be an array, not a function...

You're right, I tried to do is as an array in the first place, but wasn't able to for some reason. Put it inside the test now, just to be sure.

DerTimonius · 2022-12-13T15:59:18Z

So, I removed the conditional arguments in the tests since it does not have any effect on the executed code, now it's again this

command: 'npm access ls-collaborators test',

instead of:

command: `npm access ${testArgs[0]} test${testArgs[1]}',

I suspected that maybe this change could be the reason the tests are failing.
Also I moved the --json flag once again.

DerTimonius · 2022-12-18T12:10:43Z

Hey @sindresorhus, is there anything else I have to change?

DerTimonius added 2 commits November 15, 2022 19:36

Update util.js

7637a80

Fix npm access error

5102bdb

DerTimonius changed the title ~~Fix npm access error([#655](https://github.com/sindresorhus/np/issues/665#issue-1448125592)~~ Fix npm access error(#655) Nov 15, 2022

DerTimonius changed the title ~~Fix npm access error(#655)~~ Fix npm access error(#665) Nov 15, 2022

DerTimonius marked this pull request as ready for review November 15, 2022 21:43

sindresorhus reviewed Nov 16, 2022

View reviewed changes

source/npm/util.js Outdated Show resolved Hide resolved

Timon Jurschitsch and others added 4 commits November 16, 2022 18:51

Fix npm access subcommands in tests

2b4cf8b

Update prerequisite-tasks.js

9853c16

Fix npm access subcommands

1223221

Merge pull request #1 from DerTimonius/fix-tests

0f5af26

ar1a reviewed Nov 16, 2022

View reviewed changes

schuchard mentioned this pull request Nov 18, 2022

Running ng add @briebug/jest-schematic on an Angular 15 project throws error briebug/jest-schematic#127

Closed

Add "--json" flag

14e7788

Timon Jurschitsch and others added 3 commits November 21, 2022 08:55

Merge branch 'main' of https://github.com/DerTimonius/np

a274b88

Add --json flag to tests

e0f2564

Add --json flag to tests

abfeed0

Add checks for npm version

888bc2c

DerTimonius added 2 commits November 23, 2022 19:38

Merge branch 'main' into new-tests

bafec2a

Merge pull request #2 from DerTimonius/new-tests

a6502c5

Timon Jurschitsch added 4 commits November 23, 2022 21:30

Change npm version check to semver

0744f21

Merge branch 'main' of https://github.com/DerTimonius/np

98bd236

Remove --json flag on older npm versions

7ab1b63

Move location of --json flag

6d4f036

Move --json flag to end of argument

5d2db24

DerTimonius requested a review from sindresorhus December 13, 2022 17:00

sindresorhus merged commit 0f5dd33 into sindresorhus:main Dec 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix npm access error(#665) #666

Fix npm access error(#665) #666

DerTimonius commented Nov 15, 2022 •

edited by sindresorhus

sindresorhus commented Nov 16, 2022

sindresorhus commented Nov 16, 2022

DerTimonius commented Nov 16, 2022

DerTimonius commented Nov 16, 2022

ar1a Nov 16, 2022

schuchard commented Nov 18, 2022

DerTimonius commented Nov 18, 2022

schuchard commented Nov 21, 2022

DerTimonius commented Nov 22, 2022

karlhorky commented Nov 22, 2022 •

edited

schuchard commented Nov 22, 2022

DerTimonius commented Nov 23, 2022

DerTimonius commented Nov 27, 2022

karlhorky commented Dec 5, 2022 •

edited

karlhorky commented Dec 5, 2022 •

edited

DerTimonius commented Dec 6, 2022

DerTimonius commented Dec 13, 2022

DerTimonius commented Dec 18, 2022

Fix npm access error(#665) #666

Fix npm access error(#665) #666

Conversation

DerTimonius commented Nov 15, 2022 • edited by sindresorhus

sindresorhus commented Nov 16, 2022

sindresorhus commented Nov 16, 2022

DerTimonius commented Nov 16, 2022

DerTimonius commented Nov 16, 2022

ar1a Nov 16, 2022

Choose a reason for hiding this comment

schuchard commented Nov 18, 2022

DerTimonius commented Nov 18, 2022

schuchard commented Nov 21, 2022

DerTimonius commented Nov 22, 2022

karlhorky commented Nov 22, 2022 • edited

schuchard commented Nov 22, 2022

DerTimonius commented Nov 23, 2022

DerTimonius commented Nov 27, 2022

karlhorky commented Dec 5, 2022 • edited

karlhorky commented Dec 5, 2022 • edited

DerTimonius commented Dec 6, 2022

DerTimonius commented Dec 13, 2022

DerTimonius commented Dec 18, 2022

DerTimonius commented Nov 15, 2022 •

edited by sindresorhus

karlhorky commented Nov 22, 2022 •

edited

karlhorky commented Dec 5, 2022 •

edited

karlhorky commented Dec 5, 2022 •

edited