Skip to content

ROX-33337: Do not suppress errors during Makefile npm ci#19197

Merged
dvail merged 1 commit intomasterfrom
dv/ROX-33337-fix-error-suppression-during-ui-dep-install
Feb 25, 2026
Merged

ROX-33337: Do not suppress errors during Makefile npm ci#19197
dvail merged 1 commit intomasterfrom
dv/ROX-33337-fix-error-suppression-during-ui-dep-install

Conversation

@dvail
Copy link
Contributor

@dvail dvail commented Feb 25, 2026 •
edited
Loading

Description

The deps target runs npm ci to install dependencies, then npm audit to check for vulnerabilities. We want to ignore audit failures (which are informational) but still fail if installation errors occur.

The original command:

npm ci ... && npm audit --omit=dev || true

The || true suppressed failures from the entire chain, including npm ci. This meant installation errors were silently ignored, causing confusing downstream failures in other targets.

Solution

Wrap only npm audit in parentheses:

npm ci ... && (npm audit --omit=dev || true)

Now npm ci failures propagate correctly while npm audit failures are still ignored.

User-facing documentation

Testing and quality

  • the change is production ready: the change is GA, or otherwise the functionality is gated by a feature flag
  • CI results are inspected

Automated testing

  • added unit tests
  • added e2e tests
  • added regression tests
  • added compatibility tests
  • modified existing tests

How I validated my change

make deps passes if install completes, even with findings from npm audit:

$ make deps
....
qs  6.7.0 - 6.14.1
qs's arrayLimit bypass in comma parsing allows denial of service - https://github.com/advisories/GHSA-w7fw-mjwx-w883
fix available via `npm audit fix`
node_modules/qs

12 vulnerabilities (2 low, 5 moderate, 4 high, 1 critical)

To address issues that do not require attention, run:
  npm audit fix

To address all issues (including breaking changes), run:
  npm audit fix --force

$ echo $?
0

make deps fails if there is an error during installation:

$ make deps
cd apps/platform && npm ci --prefer-offline --fetch-timeout=60000 --no-audit --no-fund  && (npm audit --omit=dev || true)
npm error code EUSAGE
npm error
npm error `npm ci` can only install packages when your package.json and package-lock.json or npm-shrinkwrap.json are in sync. Please update your lock file with `npm install` before continuing.
npm error
npm error Missing: @apollo/client@3.14.0 from lock file
npm error Missing: @wry/caches@1.0.1 from lock file
npm error Invalid: lock file's optimism@0.17.5 does not satisfy optimism@0.18.1
npm error Missing: rehackt@0.1.0 from lock file
npm error Invalid: lock file's @wry/trie@0.4.3 does not satisfy @wry/trie@0.5.0
npm error
npm error Clean install a project
npm error
npm error Usage:
npm error npm ci
npm error
npm error Options:
npm error [--install-strategy <hoisted|nested|shallow|linked>] [--legacy-bundling]
npm error [--global-style] [--omit <dev|optional|peer> [--omit <dev|optional|peer> ...]]
npm error [--include <prod|dev|optional|peer> [--include <prod|dev|optional|peer> ...]]
npm error [--strict-peer-deps] [--foreground-scripts] [--ignore-scripts] [--no-audit]
npm error [--no-bin-links] [--no-fund] [--dry-run]
npm error [-w|--workspace <workspace-name> [-w|--workspace <workspace-name> ...]]
npm error [-ws|--workspaces] [--include-workspace-root] [--install-links]
npm error
npm error aliases: clean-install, ic, install-clean, isntall-clean
npm error
npm error Run "npm help ci" for more info
npm error A complete log of this run can be found in: /Users/dvail/.npm/_logs/2026-02-25T15_54_42_571Z-debug-0.log
make: *** [deps] Error 1

@dvail
Copy link
Contributor Author

dvail commented Feb 25, 2026

This change is part of the following stack:

Change managed by git-spice.

@openshift-ci
Copy link

openshift-ci bot commented Feb 25, 2026

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

@dvail dvail marked this pull request as ready for review February 25, 2026 15:55
@dvail dvail requested a review from a team as a code owner February 25, 2026 15:55
@rhacs-bot
Copy link
Contributor

rhacs-bot commented Feb 25, 2026

Images are ready for the commit at 0b4088e.

To use with deploy scripts, first export MAIN_IMAGE_TAG=4.11.x-97-g0b4088e22a.

@codecov
Copy link

codecov bot commented Feb 25, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 49.56%. Comparing base (300a724) to head (0b4088e).
⚠️ Report is 85 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master   #19197      +/-   ##
==========================================
+ Coverage   49.52%   49.56%   +0.03%     
==========================================
  Files        2666     2675       +9     
  Lines      201181   201820     +639     
==========================================
+ Hits        99629   100026     +397     
- Misses      94115    94336     +221     
- Partials     7437     7458      +21
Flag Coverage Δ
go-unit-tests 49.56% <ø> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dvail dvail merged commit 9c19d23 into master Feb 25, 2026
97 checks passed
@dvail dvail deleted the dv/ROX-33337-fix-error-suppression-during-ui-dep-install branch February 25, 2026 18:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pedrottimark pedrottimark pedrottimark approved these changes

Assignees

No one assigned

Labels

area/ui

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dvail @rhacs-bot @pedrottimark