Clean up console noise and enforce no-console in CI by cquirosj · Pull Request #2917 · Particular/ServicePulse

cquirosj · 2026-03-24T01:53:52Z

TODO:

test pushing a line with console.log or console.debug, and confirm the build fails

The problem

Over time, console.debug and console.log calls left behind from debugging sessions accumulated in the codebase. These had two undesirable effects:

CI output noise — the build log was flooded with debug traces when running test steps, making it harder to identify real warnings or errors at a glance.
Browser console noise — the same calls polluted the browser console in production, making it harder for end users and support teams to spot meaningful output when investigating issues.
No enforcement — there was nothing stopping new leftover calls from being merged, so the problem would keep recurring.

The challenge was fixing this without breaking the developer workflow. Developers rely on console.debug, console.log, console.info heavily during local development, and blocking that entirely would create friction with no benefit.

What changed

Removed leftover debug calls
console.debug calls used to trace auth flow steps, token lifecycle events, and auto-refresh state were removed. These were never intended to reach production and added no value outside of local debugging sessions. The same cleanup was applied to leftover calls in the test driver and test preconditions.
Introduced a thin logger abstraction
A minimal logger module was added as the single place in the codebase allowed to call console directly. Legitimate error reporting in catch blocks and error handlers — places where surfacing failures is intentional — was migrated to logger.error / logger.warn. This keeps those signals visible in both dev and production builds while satisfying the linting rule everywhere else.
Added a CI-aware no-console lint rule
The ESLint config enforces no-console as an error when CI=true (set automatically by GitHub Actions) and off locally. Turning it fully off locally means developers see no IDE warnings and can use console.debug|log|info freely during development without any friction — the rule only activates when it matters, at merge time. The build script already runs npm run lint as part of the frontend build, so no CI pipeline changes were needed.

Alternatives considered

Extending the logger with all console methods and keeping no-console always on
One alternative considered was extending logger to expose debug, log, and info — stripping those methods in production builds via import.meta.env.PROD — and enabling no-console as an error everywhere, forcing all output through the logger.

This was not pursued for two reasons:

It doesn't solve the CI test noise problem. Tests run under Vitest where import.meta.env.PROD is false, so logger.debug() would still produce output during CI test runs. Suppressing it would require adding import.meta.env.VITEST awareness to the logger — making a production module aware of the test environment, which is the wrong direction.
It couples test code to the application's logger. With no-console always on, the only escape for test files would be importing src/logger.ts — an application module — just to produce debug output. This ties test infrastructure to the System Under Test and compromises test isolation.

The current approach avoids both issues: the lint gate runs before tests execute, so noisy output never reaches the test step regardless of environment.

Reviewer Checklist

Components are broken down into sensible and maintainable sub-components.
Styles are scoped to the component using it. If multiple components need to share CSS, then a .css file is created containing the shared CSS and imported into component scoped style sections.
Naming is consistent with existing code, and adequately describes the component or function being introduced
Only functions utilizing Vue state or lifecycle hooks are named as composables (i.e. starting with 'use');
No module-level state is being introduced. If so, request the PR author to move the state to the corresponding Pinia store.

tamararivera

Great change! the output looks so much better now

tamararivera · 2026-03-24T22:06:09Z

src/Frontend/src/components/messages/FlowDiagram/FlowDiagram.vue

@@ -144,22 +145,19 @@ function constructEdges(nodes: Node<NodeData>[]): DefaultEdge[] {
        return m.receiving_endpoint !== undefined && m.sending_endpoint !== undefined && m.message_id === relatedTo && m.message_intent !== MessageIntent.Publish;
      });


Not related to this PR, but something that should be fixed: if the lint command throws an error we don't fail the script as we should, now it is failing silently: https://github.com/Particular/ServicePulse/actions/runs/23470361269/job/68291497710?pr=2917#step:5:24

That is a bug that needs to be fixed.

I think it has to do with Powershell build.ps1

The build should definitely fail, same as our C# builds fail with invalid code styles

I created #2921

tamararivera · 2026-03-24T22:08:49Z

src/Frontend/test/mocks/scenarios/index.ts

  const loader = scenarios[scenarioName];

  if (!loader) {
+    // eslint-disable-next-line no-console


Suggestion: If you delete this line, you could test that console is not allowed in the CI pipeline.

src/Frontend/src/logger.ts

johnsimons · 2026-03-24T23:33:10Z

This looks good @cquirosj, and it is a good idea to clean up all those leftovers.
Have you considered extending the logger to define all methods (debug, log, info, ...) and make the noconsole rule active for everything?
And in the logger implementation, we can filter out the debug, log, and info if it is a Production build.

johnsimons · 2026-03-24T23:37:12Z

The challenge was fixing this without breaking the developer workflow. Developers rely on console.debug, console.log, console.info heavily during local development, and blocking that entirely would create friction with no benefit.

Given the majority of developers' background at Particular is C#, I honestly don't believe replacing "console" with "logger" is going to be very challenging 😉

## What changed **Removed leftover debug calls** `console.debug` calls used to trace auth flow steps, token lifecycle events, and auto-refresh state were removed. These were never intended to reach production and added no value outside of local debugging sessions. **Introduced a thin logger abstraction** A minimal `logger` module was added as the single place in the codebase allowed to call `console` directly. Legitimate error reporting in `catch` blocks and error handlers — places where surfacing failures is intentional — was migrated to `logger.error` / `logger.warn`. This keeps those signals visible in both dev and production builds while satisfying the linting rule everywhere else. **Added a CI-aware `no-console` lint rule** The ESLint config now enforces `no-console` as an **error** when `CI=true` (set automatically by GitHub Actions) and as a **warning** locally. This means developers can still freely use `console.x` during development without any friction, but leftover calls will fail the CI build before they can merge. **Added a lint step to the CI pipeline** A `Lint Frontend` step now runs `npm run lint` before the build, so the `no-console` rule is enforced on every PR and push to `master`.

cquirosj · 2026-03-25T16:10:13Z

This looks good @cquirosj, and it is a good idea to clean up all those leftovers. Have you considered extending the logger to define all methods (debug, log, info, ...) and make the noconsole rule active for everything? And in the logger implementation, we can filter out the debug, log, and info if it is a Production build.

Good suggestion and worth considering for the future. Extending the logger with all methods and stripping debug/log/info in production builds is a cleaner, long-term architectural approach.

The reason we didn't go that route here is that this PR is specifically motivated by noise in the CI test output, not just the production browser console. Tests run under Vitest where import.meta.env.PROD is false, so logger.debug() would still output during CI test runs. We'd also need import.meta.env.VITEST in the logger to suppress it, which means a production module becomes aware of the test environment.

There's also a test isolation concern: with no-console always on, test files would be forced to import src/logger.ts (a module from the system under test) just to produce output, coupling test infrastructure to the SUT.

The current approach sidesteps both: lint runs before tests execute, so any leftover console.x fails the build before it ever produces noise in the test step. Happy to revisit the fuller logger approach when we have a need for structured logging or remote error routing.

This reverts commit 658d6e3.

internalautomation bot assigned cquirosj Mar 24, 2026

cquirosj requested a review from tamararivera March 24, 2026 02:47

tamararivera approved these changes Mar 24, 2026

View reviewed changes

johnsimons reviewed Mar 24, 2026

View reviewed changes

src/Frontend/src/logger.ts Outdated Show resolved Hide resolved

cquirosj added 3 commits March 25, 2026 09:11

Revert redundant lint step from CI workflow

91a8eb5

Fix no-console rule and remove test console noise

c872876

cquirosj force-pushed the prevent-console-log-in-ci branch from deeae45 to c872876 Compare March 25, 2026 15:11

linting

da6f3ca

cquirosj changed the title ~~Spike: Clean up console noise and enforce no-console in CI~~ Clean up console noise and enforce no-console in CI Mar 25, 2026

Fix logger type signatures to match console methods

585a671

cquirosj added 2 commits March 25, 2026 10:11

testing the build will fail

658d6e3

Revert "testing the build will fail"

132d690

This reverts commit 658d6e3.

cquirosj marked this pull request as ready for review March 25, 2026 16:56

cquirosj merged commit bcf565e into master Mar 25, 2026
5 checks passed

cquirosj deleted the prevent-console-log-in-ci branch March 25, 2026 16:57

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Clean up console noise and enforce no-console in CI#2917

Clean up console noise and enforce no-console in CI#2917
cquirosj merged 7 commits intomasterfrom
prevent-console-log-in-ci

cquirosj commented Mar 24, 2026 •

edited

Loading

Uh oh!

tamararivera left a comment

Uh oh!

tamararivera Mar 24, 2026

Uh oh!

johnsimons Mar 24, 2026

Uh oh!

johnsimons Mar 24, 2026

Uh oh!

johnsimons Mar 24, 2026

Uh oh!

tamararivera Mar 24, 2026

Uh oh!

Uh oh!

johnsimons commented Mar 24, 2026

Uh oh!

johnsimons commented Mar 24, 2026

Uh oh!

cquirosj commented Mar 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		@@ -144,22 +145,19 @@ function constructEdges(nodes: Node<NodeData>[]): DefaultEdge[] {
		return m.receiving_endpoint !== undefined && m.sending_endpoint !== undefined && m.message_id === relatedTo && m.message_intent !== MessageIntent.Publish;
		});

Conversation

cquirosj commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TODO:

The problem

What changed

Alternatives considered

Reviewer Checklist

Uh oh!

tamararivera left a comment

Choose a reason for hiding this comment

Uh oh!

tamararivera Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

johnsimons Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

johnsimons Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

johnsimons Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

tamararivera Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

johnsimons commented Mar 24, 2026

Uh oh!

johnsimons commented Mar 24, 2026

Uh oh!

cquirosj commented Mar 25, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

cquirosj commented Mar 24, 2026 •

edited

Loading