ESRP wrapper: improve logging, retries, and error handling

[Elizabeth Craig (ecraig12345)]

• If a layer fails, retry it within the tool (only if a non-fatal error) instead of relying primarily on task retries, which won't respect fatal error codes or other fatal issues. Each step now has its own retry handling as appropriate.
• Log how long each layer took
• Properly detect the current format of npm 404 errors

[Copilot]

Pull request overview

Improves ESRP npm publishing reliability and diagnostics.

Changes:

• Adds retry classification and per-layer retries.
• Improves status, 404-error, timing, and request logging.
• Updates tests, release guidance, and change metadata.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
packages/esrp-npm-release/src/utils/ReleaseError.ts	Adds retryability metadata.
packages/esrp-npm-release/src/utils/Logger.ts	Logs group duration.
packages/esrp-npm-release/src/types/api.ts	Adds ESRP release version field.
packages/esrp-npm-release/src/runRelease.ts	Implements layer retries.
packages/esrp-npm-release/src/ESRPReleaseService.ts	Improves status and npm 404 handling.
packages/esrp-npm-release/src/esrpApi/releaseHttp.ts	Classifies retryable HTTP failures.
packages/esrp-npm-release/src/esrpApi/npmRelease.ts	Formats request hashes compactly.
packages/esrp-npm-release/src/__tests__/runRelease.test.ts	Tests layer retry behavior.
packages/esrp-npm-release/src/__tests__/releaseHttp.test.ts	Tests HTTP retry classification.
packages/esrp-npm-release/src/__tests__/npmRelease.test.ts	Tests request-log formatting.
packages/esrp-npm-release/src/__tests__/ESRPReleaseService.test.ts	Tests statuses and current 404 format.
packages/esrp-npm-release/README.md	Documents internal retries.
change/@microsoft-esrp-npm-release-1b47fefb-6d1f-49e9-abac-cbc632fe2f2b.json	Records the patch change.
.ado/release.yml	Removes task-level retries.

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.

Suppressed comments (3)

.ado/release.yml:219

• Removing the task fallback leaves transient failures before layer processing with no retry path. ReleaseState.create() and ESRPReleaseService.create() run outside retryLayer, and index.ts only logs their newly retryable errors; it does not act on that flag. An exhausted/transient storage failure during initialization now immediately fails this stage, so initialization needs a classified internal retry before the task-level retry is removed.

                  - script: node "$(toolArtifactBin)"
                    displayName: '(${{ publish.groupName }}) npm publish using ESRP Release API'

packages/esrp-npm-release/src/auth/getAadToken.ts:51

• acquireTokenByClientCredential can reject for permanent AAD configuration/authentication errors (for example an invalid tenant, client, certificate, or scope), not only transient network errors. Marking every rejection retryable causes the layer runner to repeat fatal requests, contrary to the new retry contract; classify the MSAL error and set this only for transient failures.

    throw new ReleaseError(errorMessageBase, { cause: ex, retryable: true });

packages/esrp-npm-release/src/ESRPReleaseService.ts:405

• The refactor retains support for the legacy top-level errorInfo.details.errors shape, but the existing test was replaced with only the new activity-based shape. Add a parameterized case for both response formats so the compatibility branch remains protected.

  const topLevelError = (releaseStatus.errorInfo || releaseStatus.errorinfo)?.details?.errors;
  if (topLevelError && npmPublish404Pattern.test(topLevelError)) {
    return topLevelError;

[Copilot]

Pull request overview

Copilot reviewed 20 out of 20 changed files in this pull request and generated no new comments.

Suppressed comments (1)

packages/esrp-npm-release/src/utils/ReleaseState.ts:57

• This unconditionally classifies every Azure storage failure as transient. A 401/403 from invalid credentials or missing RBAC is therefore retried four times, despite the new policy that permanent or unrecognized errors fail immediately. The same unconditional classification appears in ReleaseState.ts lines 67/106 and ESRPReleaseService.ts lines 78/169/204; please centralize Azure error classification so only network failures, 408/429, and 5xx responses are retryable.

      throw new ReleaseError(`Error creating or accessing ${desc}`, { cause: err, retryable: true });

[Copilot]

Pull request overview

Copilot reviewed 23 out of 23 changed files in this pull request and generated no new comments.

Suppressed comments (4)

packages/esrp-npm-release/src/esrpApi/npmRelease.ts:153

• This replaces the first matching placeholder anywhere in the serialized message, not necessarily the generated file hash. If an earlier field such as productInfo.description equals __ESRP_FILE_HASH_0__, that field is replaced by the hash array while the actual hash remains a placeholder, corrupting the diagnostic log. Scope the replacement to the hash property.

  let formatted = JSON.stringify(redacted, null, 2);
  hashes.forEach((hash, index) => {
    formatted = formatted.replace(JSON.stringify(`__ESRP_FILE_HASH_${index}__`), JSON.stringify(hash));
  });

TEMP-npm-cli-dependency-locking.md:1

• This 213-line temporary dependency-locking research document is unrelated to the PR’s ESRP logging, retries, and npm 404 handling. Please remove it from this PR or move it to a separately described documentation change so the shipped scope matches the PR description.

# Published dependency locking for JavaScript CLI tools

.ado/release.yml:146

• This adds a sentinel tarball artifact to every production release, but the pipeline only uploads, downloads, and lists it; no step validates a scanning result. It therefore adds unrelated artifact overhead without an executable test assertion and is not part of the ESRP wrapper changes described by this PR. Please remove it or move it to a dedicated scanning change with an actual validation step.

              - script: |
                  mkdir -p "$(tgzScanTestArtifactPath)"
                  echo "This file exists only inside the tgz scan test artifact." > "$(Agent.TempDirectory)/beachball-tgz-only-sentinel.txt"
                  tar -czf "$(tgzScanTestArtifactPath)/tgz-expansion-test.tgz" \
                    -C "$(Agent.TempDirectory)" beachball-tgz-only-sentinel.txt
                  rm "$(Agent.TempDirectory)/beachball-tgz-only-sentinel.txt"
                displayName: Create tgz scanning test artifact

packages/esrp-npm-release/src/esrpApi/releaseHttp.ts:95

• Unknown exceptions are currently classified as retryable. For example, if request serialization throws before doHttpRequest creates an HttpRequestError, the outer layer retry will repeat the entire release even though this is a programmer/input failure. This also contradicts the README statement that unrecognized errors fail immediately; default unknown errors to non-retryable.

function isRetryableHttpError(error: unknown): boolean {
  return error instanceof HttpRequestError ? error.retryable : true;
}

[Copilot]

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

packages/esrp-npm-release/src/esrpApi/releaseHttp.ts:95

• Unknown errors are classified as retryable here, so failures outside HttpRequestError—for example request-body serialization or an unexpected code error—will trigger layer retries. This contradicts the documented behavior that unrecognized errors fail immediately and can repeatedly execute a broken operation. Default unknown errors to non-retryable.

function isRetryableHttpError(error: unknown): boolean {
  return error instanceof HttpRequestError ? error.retryable : true;
}

[Copilot]

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated no new comments.

Suppressed comments (3)

packages/esrp-npm-release/src/ESRPReleaseService.ts:311

• This continue retries after getReleaseStatus has already exhausted its 10 HTTP attempts, but the outer loop is bounded by 720 iterations rather than elapsed time. Since each HTTP attempt may wait 60 seconds, persistent timeouts can keep the release job polling for roughly 120 hours despite the stated 60-minute maximum. Bound polling with a wall-clock deadline (and cap each request timeout to the remaining time), or make each outer poll perform only one HTTP attempt.

          this.#logger.warn(
            `Transient error polling release ${submitReleaseResult.operationId} (will retry polling):`,
            err
          );
          continue;

packages/esrp-npm-release/src/utils/errorHelpers.ts:21

• A transient staging-token failure can surface here as the ReleaseError thrown by the getToken callback in runRelease.ts, not as a RestError. Returning false for that error discards its retryable flag, so token failures during container creation, listing, upload, or user-delegation-key acquisition bypass the new initialization/layer retry loops. Preserve an existing ReleaseError classification before checking Azure SDK errors.

export function isRetryableAzureError(error: unknown): boolean {
  if (!(error instanceof RestError)) {
    return false;
  }

packages/esrp-npm-release/src/esrpApi/npmRelease.ts:152

• Replacing the first occurrence of the bare placeholder can modify an earlier user-controlled string instead of the file hash. For example, if the product name is __ESRP_FILE_HASH_0__, the logged product name becomes the hash array while the actual hash remains a placeholder. Match the hash property as part of the replacement so logging remains accurate.

    formatted = formatted.replace(JSON.stringify(`__ESRP_FILE_HASH_${index}__`), JSON.stringify(hash));

[Copilot]

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated no new comments.