Skip to content

fix(ng-dev/release): make package deprecation errors non-fatal#3660

Merged
josephperrott merged 2 commits into
angular:mainfrom
alan-agius4:deprecate-non-fatal
May 12, 2026
Merged

fix(ng-dev/release): make package deprecation errors non-fatal#3660
josephperrott merged 2 commits into
angular:mainfrom
alan-agius4:deprecate-non-fatal

Conversation

@alan-agius4
Copy link
Copy Markdown
Contributor

Catches and logs errors that occur during npm deprecate rather than allowing them to bubble up and cause the release process to fail. This serves as a temporary workaround for issues where package deprecation requests might fail unexpectedly.

Catches and logs errors that occur during `npm deprecate` rather than allowing them to bubble up and cause the release process to fail. This serves as a temporary workaround for issues where package deprecation requests might fail unexpectedly.
@alan-agius4 alan-agius4 requested a review from josephperrott May 12, 2026 18:16
@alan-agius4 alan-agius4 added the action: merge The PR is ready for merge by the caretaker label May 12, 2026
Copy link
Copy Markdown
Member

@josephperrott josephperrott left a comment

Choose a reason for hiding this comment

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

LGTM

Your call on the comment I left, mostly just want to make sure we notice that a package failed to deprecate since it seems intermittent currently.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces error handling for the npm deprecation command by wrapping the execution in a try-catch block and logging any encountered errors. Feedback suggests improving the log output by displaying the summary message before the detailed error, including the package version for better context, and verifying the existence of the error object before logging.

Comment on lines +55 to +56
Log.error(e);
Log.error(` ✘ An error occurred while deprecating "${packageName}".`);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

It is recommended to log the summary error message before the detailed log output to improve readability for the user. Additionally, including the package version in the summary helps identify which specific version failed when multiple deprecations are performed in sequence. Finally, checking if the error output exists avoids logging empty lines when a command fails without producing output.

      Log.error("  ✘   An error occurred while deprecating \"" + packageName + "@" + version + "\".");
      if (e) {
        Log.error(e);
      }

Comment thread ng-dev/release/versioning/npm-command.ts
Co-authored-by: Joey Perrott <josephperrott@gmail.com>
Signed-off-by: Alan Agius <alan.agius4@gmail.com>
@alan-agius4 alan-agius4 force-pushed the deprecate-non-fatal branch from e4539ed to 28c12b2 Compare May 12, 2026 18:39
@josephperrott josephperrott merged commit 602e90c into angular:main May 12, 2026
12 checks passed
@josephperrott
Copy link
Copy Markdown
Member

This PR was merged into the repository. The changes were merged into the following branches:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

action: merge The PR is ready for merge by the caretaker

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants