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

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

New Issue
Open
opened 2026-06-19 18:44:10 +00:00 by spikerj · 2 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).
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
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
 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