fix: add docstrings to meet coverage threshold for SIGSEGV retry PR#4
Open
deepshekhardas wants to merge 3 commits into
Open
fix: add docstrings to meet coverage threshold for SIGSEGV retry PR#4deepshekhardas wants to merge 3 commits into
deepshekhardas wants to merge 3 commits into
Conversation
SIGSEGV was hard-classified as non-retriable in shouldRetryError on the assumption that it's always a deterministic native crash. For Node tasks that's not reliably true — many production SIGSEGVs are flaky: - Native addon races (sharp, canvas, better-sqlite3, node-rdkafka, bcrypt, etc.) — libuv thread-pool work stepping on V8 handles. Different heap layout / thread schedule on a fresh process, retry often succeeds. - JIT / GC interaction — V8 turbofan deopt or GC during a native callback. Timing-dependent. - Near-OOM in native code — when RSS approaches the cgroup limit, native allocations fail and poorly-written addons dereference NULL → SIGSEGV instead of a clean OOM-kill. A fresh process with cleaner memory often succeeds. - Host / hardware issues — bit flips, kernel quirks. Retry lands on a different host. The codebase was already inconsistent here: shouldLookupRetrySettings listed SIGSEGV as retry-config-aware, but the shouldRetryError gate short-circuited fail_run before that branch could be reached. And we already retry TASK_RUN_UNCAUGHT_EXCEPTION — clearly a user-code bug — under the user's retry policy, so refusing to retry SIGSEGV was the odd one out. Flip TASK_PROCESS_SIGSEGV from the false branch to the true branch in shouldRetryError. The existing retrying.ts pipeline then gates the retry on lockedRetryConfig + maxAttempts — same path SIGTERM and uncaught-exception already use. No new code paths; tasks without a retry policy still fail fast. Tests added in packages/core/test/errors.test.ts lock down the new classification alongside SIGTERM, SIGKILL_TIMEOUT, and the OOM codes (still non-retriable here because OOM has its own machine-bump retry path in retrying.ts that runs before shouldRetryError). Closes TRI-9234. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds JSDoc to shouldRetryError, shouldLookupRetrySettings, and correctErrorStackTrace functions to meet 80% docstring coverage threshold.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| id: release | ||
| uses: softprops/action-gh-release@v1 | ||
| if: github.event_name == 'push' | ||
| uses: softprops/action-gh-release@b4309332981a82ec1c5618f44dd2e27cc8bfbfda # v3.0.0 |
Adds JSDoc to parseError, truncateMessage, sanitizeError, taskRunErrorEnhancer, and groupTaskMetadataIssuesByTask.
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
3 participants
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.
Adds JSDoc comments to shouldRetryError, shouldLookupRetrySettings, and correctErrorStackTrace functions to bring docstring coverage up to the 80% threshold required by the project's CI checks.
Summary by cubic
Retries
TASK_PROCESS_SIGSEGVcrashes under the task’s existing retry policy and fails attempts on uncaught exceptions instead of timing out. Also adds JSDoc to key error helpers to pass the 80% doc coverage check and fixes dev workers spinning after CLI disconnect.@trigger.dev/core: Retry segfaults using the task’sretry/maxAttempts; tasks without a retry policy still fail fast.trigger.dev+@trigger.dev/core: Uncaught exceptions now fail the attempt and follow the normal retry policy (no more drift to max duration).process.disconnect.shouldRetryError,shouldLookupRetrySettings, andcorrectErrorStackTrace; CI doc coverage now exceeds 80%.Written for commit 7052c25. Summary will update on new commits.