[BUG] 7.6.0 --> 7.7.0 inc behavior change

[danchurch-alphasights]

Is there an existing issue for this?

• I have searched the existing issues

Current Behavior

We use auto in our CI to manage our versioning.

For canary releases, Auto uses a version format of <version>-canary.<pr#>.<hash>

We started to see our CI randomly failing (based on commit hash or build hash) the last few days with the following error:

Error: Running command 'npx' with args [lerna, publish, prepatch, --dist-tag, canary, --registry, <REDACT>, --yes, --no-git-reset, --no-git-tag-version, --exact, --no-verify-access, --preid, canary.661.2207bf7] failed


Changes:
 - <REDACT>: 0.127.2 => null
 - <REDACT>: 0.35.0 => null

...

lerna ERR! Cannot read properties of null (reading 'trim')
lerna ERR! errno "undefined" is not a valid exit code - exiting with code 1

After digging deep into Lerna / Auto, I found out the root cause was due to a change of the semver package between 7.6.0 and 7.7.0

	semver.inc('1.0.0', 'prepatch', 'canary.661.2207bf')	semver.valid('1.0.1-canary.661.2207bf.0')
7.6.0	1.0.1-canary.661.2207bf.0	1.0.1-canary.661.2207bf.0
7.7.0	null	1.0.1-canary.661.2207bf.0

Expected Behavior

I would expect semver.inc('1.0.0', 'prepatch', 'canary.661.2207bf') to continue to return a valid value or semver.valid('1.0.1-canary.661.2207bf.0') to return null

Steps To Reproduce

npm i semver@7.6.0 --save-exact

node -e "console.log(require('semver').inc('1.0.0', 'prepatch', 'canary.661.2207bf'));"

1.0.1-canary.661.2207bf.0

---

npm i semver@7.7.0 --save-exact

node -e "console.log(require('semver').inc('1.0.0', 'prepatch', 'canary.661.2207bf'));"

null

---

npm i semver@7.7.0 --save-exact

node -e "console.log(require('semver').valid('1.0.1-canary.661.2207bf.0'));"

1.0.1-canary.661.2207bf.0

Environment

• npm: 8.3.1
• Node: v16.14.0
• OS: Mac Sonoma 14.7.2
• platform: darwin

[wraithgar]

Looks like there's something unexpected going on w/ our regexes. Both the Semver class constructor and .inc use the same regex to validate prereleases. This should work, and finding out why it doesn't is the next step.

constructor:

    const m = version.trim().match(options.loose ? re[t.LOOSE] : re[t.FULL])

    if (!m) {
      throw new TypeError(`Invalid Version: ${version}`)
    }

inc:

      // Avoid an invalid semver results
      if (identifier) {
        const match = `-${identifier}`.match(this.options.loose ? re[t.PRERELEASELOOSE] : re[t.PRERELEASE])
        if (!match || match[1] !== identifier) {
          throw new Error(`invalid identifier: ${identifier}`)
        }
      }

regex:

createToken('FULLPLAIN', `v?${src[t.MAINVERSION]
}${src[t.PRERELEASE]}?${
  src[t.BUILD]}?`)

createToken('FULL', `^${src[t.FULLPLAIN]}$`)

re[t.FULL]:

/^v?(0|[1-9]\d{0,256})\.(0|[1-9]\d{0,256})\.(0|[1-9]\d{0,256})(?:-((?:0|[1-9]\d{0,256}|\d{0,256}[a-zA-Z-][a-zA-Z0-9-]{0,250})(?:\.(?:0|[1-9]\d{0,256}|\d{0,256}[a-zA-Z-][a-zA-Z0-9-]{0,250}))*))?(?:\+([a-zA-Z0-9-]{1,250}(?:\.[a-zA-Z0-9-]{1,250})*))?$/

re[t.PRERELEASE]:

/(?:-((?:0|[1-9]\d{0,256}|\d{0,256}[a-zA-Z-][a-zA-Z0-9-]{0,250})(?:\.(?:0|[1-9]\d{0,256}|\d{0,256}[a-zA-Z-][a-zA-Z0-9-]{0,250}))*))/

regex is doing something unexpected here.

re.PRERELEASE

> '-canary.661.2207bf'.match(/(?:-((?:0|[1-9]\d{0,256}|\d{0,256}[a-zA-Z-][a-zA-Z0-9-]{0,250})(?:\.(?:0|[1-9]\d{0,256}|\d{0,256}[a-zA-Z-][a-zA-Z0-9-]{0,250}))*))/)
[
  '-canary.661.2207',
  'canary.661.2207',
  index: 0,
  input: '-canary.661.2207bf',
  groups: undefined
]
> 

You'll see it's truncating the bf from the end of the prerelease id.

re.FULL

> '1.0.0-canary.661.2207bf'.match(/^v?(0|[1-9]\d{0,256})\.(0|[1-9]\d{0,256})\.(0|[1-9]\d{0,256})(?:-((?:0|[1-9]\d{0,256}|\d{0,256}[a-zA-Z-][a-zA-Z0-9-]{0,250})(?:\.(?:0|[1-9]\d{0,256}|\d{0,256}[a-zA-Z-][a-zA-Z0-9-]{0,250}))*))?(?:\+([a-zA-Z0-9-]{1,250}(?:\.[a-zA-Z0-9-]{1,250})*))?$/)
[
  '1.0.0-canary.661.2207bf',
  '1',
  '0',
  '0',
  'canary.661.2207bf',
  undefined,
  index: 0,
  input: '1.0.0-canary.661.2207bf',
  groups: undefined
]

But it's not here.

It appears that when t.PREELEASE is being rolled into t.FULL something is altering its behavior. Some regex digging is in order now.

[wraithgar]

1a is a much simpler prerelease identifier that reproduces the same bug.

> '-1a'.match(/(?:-((?:0|[1-9]\d{0,256}|\d{0,256}[a-zA-Z-][a-zA-Z0-9-]{0,250})(?:\.(?:0|[1-9]\d{0,256}|\d{0,256}[a-zA-Z-][a-zA-Z0-9-]{0,250}))*))?/)
[ '-1', '1', index: 0, input: '-1a', groups: undefined ]

> '1.0.0-1a'.match(/^v?(0|[1-9]\d{0,256})\.(0|[1-9]\d{0,256})\.(0|[1-9]\d{0,256})(?:-((?:0|[1-9]\d{0,256}|\d{0,256}[a-zA-Z-][a-zA-Z0-9-]{0,250})(?:\.(?:0|[1-9]\d{0,256}|\d{0,256}[a-zA-Z-][a-zA-Z0-9-]{0,250}))*))?(?:\+([a-zA-Z0-9-]{1,250}(?:\.[a-zA-Z0-9-]{1,250})*))?$/)
[
  '1.0.0-1a',
  '1',
  '0',
  '0',
  '1a',
  undefined,
  index: 0,
  input: '1.0.0-1a',
  groups: undefined
]

[wraithgar]

Solved: the ^ and $ are load bearing.

> '-1a'.match(/(?:-((?:0|[1-9]\d{0,256}|\d{0,256}[a-zA-Z-][a-zA-Z0-9-]{0,250})(?:\.(?:0|[1-9]\d{0,256}|\d{0,256}[a-zA-Z-][a-zA-Z0-9-]{0,250}))*))?/)
[ '-1', '1', index: 0, input: '-1a', groups: undefined ]
> '-1a'.match(/^(?:-((?:0|[1-9]\d{0,256}|\d{0,256}[a-zA-Z-][a-zA-Z0-9-]{0,250})(?:\.(?:0|[1-9]\d{0,256}|\d{0,256}[a-zA-Z-][a-zA-Z0-9-]{0,250}))*))?$/)
[ '-1a', '1a', index: 0, input: '-1a', groups: undefined ]

PR incoming

[danchurch-alphasights]

Thank you @wraithgar!