spikerj/spikersoft-issues
1
0 Fork 0
Code Issues 23 Pull Requests Actions Packages Projects Releases Wiki Activity

[Bug] SonarQube S7059: async operation inside a constructor #242

New Issue
Closed
opened 2026-06-19 18:44:10 +00:00 by spikerj · 3 comments
spikerj commented 2026-06-19 18:44:10 +00:00
Owner
Copy link
Copy source

Surfaced by SonarQube (learn.spikersoft.com), rule typescript:S7059, severity CRITICAL (RELIABILITY).

Problem

Starting asynchronous work in a constructor means the object is used before async init completes, and rejections are unhandled. Move the async work to an explicit init method / factory / lifecycle hook (e.g. ngOnInit).

Locations

  • projects/spikersoft/src/app/_components/learning/git-playground/git-playground-runtime.service.ts:43
  • libraries/features/dev-tools-x86-playground/src/lib/services/blink.ts:278
  • libraries/features/dev-tools-x86-playground/src/lib/assembly-playground.component.ts:213
  • libraries/platform/activity-tracking/src/lib/activity-tracking.service.ts:87

Fix

Extract the awaited work into an async init()/ngOnInit() (or a static async factory) and ensure callers await it; handle rejections.

Rule: https://rules.sonarsource.com/typescript/RSPEC-7059/

Surfaced by SonarQube (`learn.spikersoft.com`), rule `typescript:S7059`, severity CRITICAL (RELIABILITY). ## Problem Starting asynchronous work in a constructor means the object is used before async init completes, and rejections are unhandled. Move the async work to an explicit init method / factory / lifecycle hook (e.g. `ngOnInit`). ## Locations - `projects/spikersoft/src/app/_components/learning/git-playground/git-playground-runtime.service.ts:43` - `libraries/features/dev-tools-x86-playground/src/lib/services/blink.ts:278` - `libraries/features/dev-tools-x86-playground/src/lib/assembly-playground.component.ts:213` - `libraries/platform/activity-tracking/src/lib/activity-tracking.service.ts:87` ## Fix Extract the awaited work into an async `init()`/`ngOnInit()` (or a static async factory) and ensure callers await it; handle rejections. _Rule: https://rules.sonarsource.com/typescript/RSPEC-7059/_
spikerj added the bugsonarqube labels 2026-06-19 18:44:10 +00:00
spikerj commented 2026-06-20 02:04:03 +00:00
Author
Owner
Copy link
Copy source

Deferred (ticket stays open). On review, the four flagged sites are intentional patterns rather than accidental async-in-ctor:

  • activity-tracking.service.ts:87 — deliberate void this.manifestService.ensureLoaded() background fetch (heavily commented; gated on auth, safe no-op for anonymous).
  • git-playground-runtime.service.ts:43 — void this.refreshTerminalTranslations() fire-and-forget i18n refresh.
  • blink.ts:278 — idiomatic boot-promise capture (this.bootPromise = this.#initEmscripten(mode)) so callers can await readiness.
  • assembly-playground.component.ts:213 — void this.lessonRunner.loadCatalog() alongside an existing afterNextRender block.

Properly satisfying S7059 means moving this work to explicit init methods / static factories, which changes init/timing semantics for i18n, the manifest fetch and the Emscripten boot — needs full-app runtime verification, so it's split into a focused follow-up rather than a risky mechanical edit.

Deferred (ticket stays open). On review, the four flagged sites are intentional patterns rather than accidental async-in-ctor: - `activity-tracking.service.ts:87` — deliberate `void this.manifestService.ensureLoaded()` background fetch (heavily commented; gated on auth, safe no-op for anonymous). - `git-playground-runtime.service.ts:43` — `void this.refreshTerminalTranslations()` fire-and-forget i18n refresh. - `blink.ts:278` — idiomatic boot-promise capture (`this.bootPromise = this.#initEmscripten(mode)`) so callers can await readiness. - `assembly-playground.component.ts:213` — `void this.lessonRunner.loadCatalog()` alongside an existing `afterNextRender` block. Properly satisfying `S7059` means moving this work to explicit init methods / static factories, which changes init/timing semantics for i18n, the manifest fetch and the Emscripten boot — needs full-app runtime verification, so it's split into a focused follow-up rather than a risky mechanical edit.
spikerj commented 2026-07-29 18:34:31 +00:00
Author
Owner
Copy link
Copy source

Audited against origin/master — STILL PRESENT, and worth splitting: the finding set is mostly false positives.

Sync-over-async blocking calls in non-test code, by category:

1 — a genuine production instance: SpikerSoft.Api/Extensions/ServiceCollectionExtensions.cs:812 — connectionFactory.CreateConnectionAsync().GetAwaiter().GetResult() inside DI registration. This is the real one: blocking on a RabbitMQ connection during service construction, on the startup path. It's also the same connection-creation call that #886 flags as having no retry/backoff — so both tickets touch this line, and fixing them together makes sense (make it async-safe and give it a retry ladder).

2 — disposal, which is defensible: SpikerSoft.Business.Scheduling/Services/ScheduledTaskPublisher.cs:179 and :181 — _channel?.CloseAsync().GetAwaiter().GetResult() and the same for _connection. Blocking in a synchronous Dispose() is the conventional pattern when IAsyncDisposable isn't plumbed through. Worth converting to IAsyncDisposable if the call sites allow it, but it isn't the constructor-async smell this rule targets.

3 — five that should be excluded, not fixed: SpikerSoft.Business.CodeExecution/Domain/Lessons/Curriculum/Tier15_Async/Lesson1500_TaskBasics.cs:49, Lesson1501_AsyncAwait.cs:48, Lesson1502_WhenAll.cs:50, Lesson1503_Cancellation.cs:47 and :57.

Those last five are teaching material — the Tier-15 async curriculum, where GetAwaiter().GetResult() is being demonstrated to students in a grading harness. "Fixing" them would damage the lessons. They're the same shape as the S4325 finding class already accepted as a strict-mode artifact in the frontend triage.

Suggested disposition: fix ServiceCollectionExtensions.cs:812 (ideally with #886), consider IAsyncDisposable for the publisher, and add a path exclusion for Domain/Lessons/Curriculum/** to the SonarQube config rather than re-triaging those five on every scan. The mechanism already exists — sonar-scan.yml:75 uses sonar.issue.ignore.multicriteria with three entries, so a fourth scoped to the curriculum tree is three lines.

That exclusion is worth doing regardless of this ticket: the curriculum tree deliberately contains anti-patterns as pedagogy, and it will keep generating findings across many rules (#643's noise-policy decision is the natural place to record it).

Audited against `origin/master` — **STILL PRESENT, and worth splitting: the finding set is mostly false positives.** Sync-over-async blocking calls in non-test code, by category: **1 — a genuine production instance:** `SpikerSoft.Api/Extensions/ServiceCollectionExtensions.cs:812` — `connectionFactory.CreateConnectionAsync().GetAwaiter().GetResult()` inside DI registration. This is the real one: blocking on a RabbitMQ connection during service construction, on the startup path. It's also the same connection-creation call that **#886** flags as having no retry/backoff — so both tickets touch this line, and fixing them together makes sense (make it async-safe *and* give it a retry ladder). **2 — disposal, which is defensible:** `SpikerSoft.Business.Scheduling/Services/ScheduledTaskPublisher.cs:179` and `:181` — `_channel?.CloseAsync().GetAwaiter().GetResult()` and the same for `_connection`. Blocking in a synchronous `Dispose()` is the conventional pattern when `IAsyncDisposable` isn't plumbed through. Worth converting to `IAsyncDisposable` if the call sites allow it, but it isn't the constructor-async smell this rule targets. **3 — five that should be excluded, not fixed:** `SpikerSoft.Business.CodeExecution/Domain/Lessons/Curriculum/Tier15_Async/Lesson1500_TaskBasics.cs:49`, `Lesson1501_AsyncAwait.cs:48`, `Lesson1502_WhenAll.cs:50`, `Lesson1503_Cancellation.cs:47` and `:57`. Those last five are **teaching material** — the Tier-15 async curriculum, where `GetAwaiter().GetResult()` is being demonstrated to students in a grading harness. "Fixing" them would damage the lessons. They're the same shape as the `S4325` finding class already accepted as a strict-mode artifact in the frontend triage. **Suggested disposition:** fix `ServiceCollectionExtensions.cs:812` (ideally with #886), consider `IAsyncDisposable` for the publisher, and add a **path exclusion for `Domain/Lessons/Curriculum/**`** to the SonarQube config rather than re-triaging those five on every scan. The mechanism already exists — `sonar-scan.yml:75` uses `sonar.issue.ignore.multicriteria` with three entries, so a fourth scoped to the curriculum tree is three lines. That exclusion is worth doing regardless of this ticket: the curriculum tree deliberately contains anti-patterns as pedagogy, and it will keep generating findings across many rules (**#643**'s noise-policy decision is the natural place to record it).
spikerj commented 2026-08-07 13:50:32 +00:00
Author
Owner
Copy link
Copy source

Closing 2026-08-07 as no-longer-applicable — the finding set was explicitly accepted on the SonarQube server, so there is nothing left to fix or to migrate.

Live (SonarQube API, learn.spikersoft.com, last analysis 2026-08-07T11:49Z): typescript:S7059 has 0 OPEN/CONFIRMED issues. All five findings the rule ever raised are dispositioned:

file status dispositioned
.../git-playground/git-playground-runtime.service.ts:43 RESOLVED / WONTFIX 2026-07-17
libraries/features/dev-tools-x86-playground/src/lib/assembly-playground.component.ts:224 RESOLVED / WONTFIX 2026-07-17
libraries/platform/activity-tracking-impl/src/lib/activity-tracking.service.ts:76 RESOLVED / WONTFIX 2026-07-20
libraries/features/dev-tools-x86-playground/src/lib/services/blink.ts:278 RESOLVED / WONTFIX 2026-07-20
libraries/features/art-studio/.../art-usage-meter.component.ts CLOSED / FIXED 2026-07-19

The accept comments on the server record exactly the reasoning from this ticket's 2026-06-20 comment — "intentional fire-and-forget background init, explicitly marked with void (and documented in comments)", and for blink.ts "constructor starts async Emscripten init and exposes it as the public awaitable bootPromise; the ctor stays sync and nothing is fire-and-forgotten."

Code: spikersoft-angular@8e5a4048 — the four patterns are still present and still deliberate (git-playground-runtime.service.ts:43, assembly-playground.component.ts:224, activity-tracking-impl/.../activity-tracking.service.ts:76 with a 9-line comment explaining the gating, blink.ts:270,278 where bootPromise is a public readonly awaitable). Nothing was silently pinned or exempted at the scanner level — the rule is still active in the TypeScript quality profile, so a genuine new async-in-constructor would still be raised.

One housekeeping note before this closes. The 2026-07-29 comment on this ticket is misfiled: it audits C# sync-over-async (GetAwaiter().GetResult()) in spikersoft-backend, not typescript:S7059. Its substance is not lost — the one genuine production instance it names, SpikerSoft.Api/Extensions/ServiceCollectionExtensions.cs:812 (connectionFactory.CreateConnectionAsync().GetAwaiter().GetResult()), is still on backend master today and is the same line tracked by #886 (RabbitMQ connection with no retry/backoff), which remains open. Its second suggestion — a sonar.issue.ignore.multicriteria path exclusion for Domain/Lessons/Curriculum/**, where GetAwaiter().GetResult() is deliberate teaching material — belongs with #643's noise-policy decision, which is where I have recorded it.

Not migrated: the frontend work this ticket describes was declined on the record, and the findings are closed at the source.

— Opus 5 Agent

Closing 2026-08-07 as no-longer-applicable — the finding set was explicitly accepted on the SonarQube server, so there is nothing left to fix or to migrate. **Live (SonarQube API, `learn.spikersoft.com`, last analysis 2026-08-07T11:49Z):** `typescript:S7059` has **0 OPEN/CONFIRMED issues**. All five findings the rule ever raised are dispositioned: | file | status | dispositioned | |---|---|---| | `.../git-playground/git-playground-runtime.service.ts:43` | RESOLVED / **WONTFIX** | 2026-07-17 | | `libraries/features/dev-tools-x86-playground/src/lib/assembly-playground.component.ts:224` | RESOLVED / **WONTFIX** | 2026-07-17 | | `libraries/platform/activity-tracking-impl/src/lib/activity-tracking.service.ts:76` | RESOLVED / **WONTFIX** | 2026-07-20 | | `libraries/features/dev-tools-x86-playground/src/lib/services/blink.ts:278` | RESOLVED / **WONTFIX** | 2026-07-20 | | `libraries/features/art-studio/.../art-usage-meter.component.ts` | CLOSED / FIXED | 2026-07-19 | The accept comments on the server record exactly the reasoning from this ticket's 2026-06-20 comment — *"intentional fire-and-forget background init, explicitly marked with `void` (and documented in comments)"*, and for `blink.ts` *"constructor starts async Emscripten init and exposes it as the public awaitable `bootPromise`; the ctor stays sync and nothing is fire-and-forgotten."* **Code:** `spikersoft-angular@8e5a4048` — the four patterns are still present and still deliberate (`git-playground-runtime.service.ts:43`, `assembly-playground.component.ts:224`, `activity-tracking-impl/.../activity-tracking.service.ts:76` with a 9-line comment explaining the gating, `blink.ts:270,278` where `bootPromise` is a public readonly awaitable). Nothing was silently pinned or exempted at the scanner level — the rule is still **active** in the TypeScript quality profile, so a genuine new async-in-constructor would still be raised. **One housekeeping note before this closes.** The 2026-07-29 comment on this ticket is misfiled: it audits **C# sync-over-async** (`GetAwaiter().GetResult()`) in `spikersoft-backend`, not `typescript:S7059`. Its substance is not lost — the one genuine production instance it names, `SpikerSoft.Api/Extensions/ServiceCollectionExtensions.cs:812` (`connectionFactory.CreateConnectionAsync().GetAwaiter().GetResult()`), is still on backend master today and is the same line tracked by **#886** (RabbitMQ connection with no retry/backoff), which remains open. Its second suggestion — a `sonar.issue.ignore.multicriteria` path exclusion for `Domain/Lessons/Curriculum/**`, where `GetAwaiter().GetResult()` is deliberate teaching material — belongs with **#643**'s noise-policy decision, which is where I have recorded it. Not migrated: the frontend work this ticket describes was declined on the record, and the findings are closed at the source. — Opus 5 Agent
Sign in to join this conversation.
No Branch/Tag Specified
Labels
Clear labels
agentic
Actionable surfaced by the agentic architecture & security review
 bug
Something is not working
 duplicate
This issue or pull request already exists
 enhancement
New feature
 epic
Cross-repo epic — children live in per-repo trackers
 help wanted
Need some help
 invalid
Something is wrong
 question
More information is needed
 sonarqube
Code quality issue surfaced by SonarQube (bug / code smell)
 wontfix
This won't be fixed
No labels bug sonarqube
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
spikerj (Joseph Spiker)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: spikerj/spikersoft-issues#242