[dev-tool] Surface actual failures in extract-api and test runner#39238
Open
jeremymeng wants to merge 4 commits into
Open
[dev-tool] Surface actual failures in extract-api and test runner#39238jeremymeng wants to merge 4 commits into
jeremymeng wants to merge 4 commits into
Conversation
Previously several dev-tool commands could exit non-zero with no
actionable message, making CI "Build libraries" failures hard to
diagnose. This surfaces the real errors:
- extract-api ignored api-extractor's result. Now captures
{succeeded, errorCount, warningCount}, throws with the entry name,
mainEntryPoint, counts and expected report path before the downstream
ENOENT, and folds failure into `success`.
- Top-level catch now handles non-Error throws (Error -> stack,
object -> JSON, else -> String), prints err.cause, and removes the
misleading console.trace that printed the callback's own stack.
- The vitest runner's concurrently rejection is an array of close-event
objects, not an Error. Now summarized via summarizeCloseEvents and
returned as a clean exit-1 with a message.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Improves dev-tool diagnostics by surfacing underlying API extraction and test-runner failures.
Changes:
- Propagates API Extractor results and detailed failures.
- Formats non-
Errortop-level rejections and causes. - Summarizes failed Vitest subprocesses and returns failure status.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
common/tools/dev-tool/src/util/testUtils.ts |
Summarizes subprocess failures and preserves proxy cleanup. |
common/tools/dev-tool/src/index.ts |
Formats top-level errors and causes. |
common/tools/dev-tool/src/commands/run/testVitest.ts |
Reports non-proxy Vitest failures cleanly. |
common/tools/dev-tool/src/commands/run/extract-api.ts |
Validates and propagates API Extractor results. |
Intermittent Windows nightly failures showed extract-api crashing with no message and no turbo summary, dying at the final merged-API writeFile in buildMergedApiJson. Wrap the FS operations in writeRuntimeApiFiles and buildMergedApiJson with a transient-error retry helper (EBUSY/EPERM/ENOENT/ EMFILE/EAGAIN/UNKNOWN), and guard the two call sites so a post-retry failure sets success = false and logs a file-qualified error. This lets extract-api exit non-zero gracefully so turbo can attribute the failing task, and lets most transient flakes self-heal. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
- index.ts: gate the error-cause log on `!== undefined` so valid falsy causes (0, false, "", null) are still surfaced. - testUtils.ts: concurrently CloseEvent.exitCode is string|number where a string is the terminating signal; type it accordingly and label signal terminations distinctly from numeric exit codes. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Several
dev-toolcommands — most notablyextract-api— could exit non-zero with no actionable message. Failures surfaced only as the genericErrors occurred. See the output above.(exit 1) or[Internal Error] undefined, which made CI Build libraries failures very hard to diagnose (anextract-apistep failing with exit code 1 and no clear error).This PR makes dev-tool surface the real underlying failure in three spots.
Changes
Gap 1 —
extract-apiignored api-extractor's resultcommands/run/extract-api.tspreviously calledextractApi(...)and discarded its return value, so a genuine api-extractor failure only manifested later as a bareENOENTwhen reading the (never-written) temp report, and the exports branch keptsuccess = true. Now:extractApireturns{ succeeded, errorCount, warningCount }.extractApiForEntrythrows with the entry name/runtime,mainEntryPointFilePath, error/warning counts, and the expected report path before the downstreamreadFile..succeededintosuccess.Gap 4 — top-level catch mishandled non-Error throws
index.tsassumederrwas anError(err.message/console.trace(err.stack)), yielding[Internal Error] undefinedfor non-Error rejections and printing the callback's own stack rather than the error's. Now aformat(value)helper handlesError → stack ?? message,object → JSON.stringify(guarded), elseString, printserr.causewhen present, and drops the misleadingconsole.trace. Still exits 255.Gap 5 — vitest runner's
concurrentlyrejection is not an Errorconcurrently([...]).resultrejects with an array of close-event objects, not anError, so a failing test run propagated to the top-level catch as[Internal Error] undefined. AddedsummarizeCloseEvents(inutil/testUtils.ts) that extracts the non-zeroexitCodeevents into a readable message (e.g.vitest exited with code 1). Bothconcurrently(...).resultawait sites (testVitest.tsnon-proxy path andrunTestsWithProxyTool) now catch, log an actionable message, andreturn false(clean exit 1 with a message).Note for reviewers
The
extract-apigenuine-failure path now surfaces as exit 255 (a thrownErrorrouted through the top-level catch) rather than the previous message-less exit 1. This is intentional — the throw carries the actionable context.Verification
pnpm build -F @azure/dev-tool→ success;tsc --noEmitincommon/tools/dev-tool→ clean.sdk/alertrulerecommendations/arm-alertrulerecommendationsand confirmed clear output:ae-forgotten-exporttoerror):API Extractor failed for entry 'node' (mainEntryPoint: ...\dist\esm\index.d.ts) with 1 errors and 1 warnings. Expected report at ...\review\arm-alertrulerecommendations-node.api.md.→ exit 255.The "mainEntryPointFilePath" path does not exist: ...error with a proper stack → exit 255..d.ts(internal extractor crash): full real stack under[Internal Error]→ exit 255.