WIP: fix errors in migration#124
Conversation
SummarySummaryThis PR fixes 5 bugs discovered during migration testing and log analysis, applied consistently across all 4 SonarQube version pipelines (sq-9.9, sq-10.0, sq-10.4, sq-2025). Bug 1: Hotspot comment API parameter mismatch — The code sent Bug 2: PDF VFS data resolution — Font data lookup path was incorrect, causing Bug 3: PDF PdfPrinter constructor — pdfmake 0.3.x requires 3 parameters including Bug 4: Lost error details in logs — When projects failed, logs only showed step names, not error reasons. Bug 5: "Newer report already processed" crash — When SonarCloud rejected analysis because a newer report existed, the entire project was marked failed. This breaks resume because the project's analysis is valid—SonarCloud just won't process a duplicate. Verification: After these fixes, 3/3 test projects succeeded with 0 errors and all PDF reports generated successfully. What reviewers should knowWhere to Start
Key Patterns to WatchAPI Parameter Fix — All 4 Error Message Enhancement — All "Newer Report" Handling — New code treats "a newer report has already been processed" as a success (log warning, return task). This assumes SonarCloud's existing analysis is valid and can be used for data sync.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 21e91c8. Configure here.
![sonar-review-alpha[bot]](https://avatars.githubusercontent.com/in/3025063?s=60&v=4){kind=small}
| const reason = task.errorMessage || task.errorType || 'Unknown error (check SonarCloud CE task ' + ceTaskId + ')'; | ||
| if (reason.includes('a newer report has already been processed')) { | ||
| logger.warn(`Analysis rejected (newer report exists) — treating as success for migration resume`); | ||
| return task; | ||
| } | ||
| throw new SonarCloudAPIError(`Analysis ${task.status.toLowerCase()}: ${reason}`); |
There was a problem hiding this comment.
Logic duplication: This 5-line block — build reason, detect the "newer report" message, warn and return — is copy-pasted verbatim into all four CE helpers:
sq-9.9/sonarcloud/api-client/helpers/wait-for-analysis.jssq-10.4/sonarcloud/api-client/helpers/ce-methods.jssq-2025/sonarcloud/api-client/helpers/ce-task-methods.js
The logic is completely version-agnostic (same task, ceTaskId, reason variables, same string match, same return). If SonarCloud ever changes the error text or you want to adjust the warning, all four files need editing.
src/shared/ already exists for cross-pipeline utilities. Extract a helper there — e.g. src/shared/utils/ce-task-helpers.js exporting buildCeFailureReason(task, ceTaskId) and isNewerReportError(reason) — and call it from each pipeline. The pipeline files keep their throw/return control flow, they just delegate the detection logic to the shared helper.
- Mark as noise
| const reason = task.errorMessage || task.errorType || 'Unknown error (check SonarCloud CE task ' + ceTaskId + ')'; | ||
| if (reason.includes('a newer report has already been processed')) { | ||
| logger.warn(`Analysis rejected (newer report exists) — treating as success for migration resume`); | ||
| return task; | ||
| } | ||
| throw new SonarCloudAPIError(`Analysis ${task.status.toLowerCase()}: ${reason}`); | ||
| } | ||
| if (Date.now() - startTime > maxWaitMs) throw new SonarCloudAPIError(`Analysis timeout after ${maxWaitSeconds} seconds`); |
There was a problem hiding this comment.
When the "newer report" path fires, this function returns task with task.status === 'FAILED'. Every caller in the upload chain does:
const result = await client.waitForAnalysis(ceTask.id, maxWaitSeconds);
logger.info('Analysis completed successfully'); // always runs
return result;Two consequences:
- The "Analysis completed successfully" info log fires unconditionally, so in the "newer report" case the logs show a warning immediately followed by a success message — confusing when reading logs post-migration.
- The returned
result.statusis'FAILED'. If any caller further up the chain branches onresult.status, it will misclassify this as a real failure.
The fix in the CHANGELOG says 3/3 projects succeeded, so current callers are clearly not checking result.status — but this is a latent trap. Consider either returning a synthetic success-shaped object ({ ...task, status: 'SUCCESS' }) or documenting the contract explicitly.
- Mark as noise
Bug 1: Batch distributor drops issues (Critical) The batch distributor split >5K issues across multiple sequential analyses. But SonarCloud's issue tracker treats each analysis as a complete snapshot — it closes issues from the previous analysis that don't appear in the current one. Only the last batch's issues survived. Angular: 31,642 issues → only ~1,642 survived (95% data loss) Fix: Disabled batch distribution in should-batch.js. All issues now upload in a single analysis. Bug 2: Wrong issueStatuses for SQ 2025 pipeline The SQ 2025 API doesn't accept CLOSED as an issueStatuses value (valid: OPEN,CONFIRMED,FALSE_POSITIVE,ACCEPTED,FIXED,IN_SANDBOX). Updated both issue-methods.js and issues-hotspots.js. Verified results: Project SQ Server SQ Cloud Status Nodejs (15 issues) 15 15 EXACT Angular (31,642 issues) 31,642 31,641 EXACT (-1) MuleSoft (1,278 violations) 1,278 pending sync* ON TRACK The migration is still running (metadata sync for Angular's 9,981 matched issues). The batch upload phase completed successfully for all projects.
2 participants
Note
High Risk
High risk because it changes core migration behavior (disables issue batching) and alters how CE task failures are interpreted during resume, which can affect data correctness and masking of real failures. Also adds a script that can delete all SonarCloud projects in an org if misused.
Overview
Prevents large-project issue data loss by disabling issue batch distribution entirely (
shouldBatch()now always returnsfalse), forcing uploads to happen as a single analysis.Improves migration resilience and debuggability by (1) treating SonarCloud CE failures containing "a newer report has already been processed" as a resume-success case across pipelines, and (2) enriching failure logs (
recordProjectOutcome) with per-step error details plus better CE failure reasons (errorMessage→ fallback toerrorType/task id).Fixes SonarCloud hotspot comment writes by sending the correct parameter (
commentinstead oftext), and fixes PDF generation by updating thepdfmakeprinter setup to includeURLResolver/writeFileSyncand handle ESM VFS exports.Adds/adjusts debugging utilities: a script that deletes all projects in a SonarCloud org, splits combined migrate+verify into separate scripts, and updates
.gitignoreto track new top-level docs (CLAUDE.md,DESIGN.md).Reviewed by Cursor Bugbot for commit 3adc3cc. Bugbot is set up for automated code reviews on this repo. Configure here.