Repository navigation
Conversation
Both samples in docs/runtime/utils.mdx peek at a rejected promise and never handle it. bun test reports the unhandled rejection as the failure of the test, so both samples fail when run as printed. Handle the rejection after the assertions. The first sample also read .message from the peek() result, which does not type-check (the result is typed Promise<never>). Assert on the value with toBeInstanceOf and toHaveProperty.
Collaborator
Author
|
Status
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Essentials Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 0 remain after this review. WalkthroughThe documentation updates ChangesBun.peek rejection documentation
Priority: ⬇️ Low Merge Risk: ⚪ Minimal · up to The documentation examples address rejected promises and type-checking issues without introducing merge-blocking risk. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Comment |
This branch has not been deployed
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
Bun.peektest samples indocs/runtime/utils.mdxfail when run withbun testas printed. The output iserror: Successfully tested promise rejection,(fail) peekanderror: oh nooo,(fail) peek.status.Bun.peekandpeek.statusdo not mark a promise as handled, sobun testreports the unhandled rejection as the failure of the test.Property 'message' does not exist on type 'Promise<never>'.Fix
rejected.catch(() => {});after the assertions in both samples, with a comment that gives the reason.expect(peek(rejected).message)withtoBeInstanceOf(Error)andtoHaveProperty("message", ...). These assertions need no cast.tscagainstpackages/bun-typesreports no errors.test/js/bun/util/peek.test.tsalready handles its rejection the same way.Background
Bun.peek(promise)reads the result of a settled promise withoutawait. For a rejected promise it returns the rejection reason.bun testattributes it to the test that runs at that time and fails that test (docs/test/runtime-behavior.mdx, "Promise Rejections").peek<T>(promise: T | Promise<T>): Promise<T> | T, so the result forPromise.reject(...)has the typePromise<never>.Notes
Before, the two samples as printed on main, on bun 1.4.2 (744846f):
1.4.3-canary.1 (c6b7fcb) gives the same result, so the failure is not new. The samples date from #2148.
After, the two samples as printed on this branch, on both builds:
Type check. I linked
packages/bun-typesinto a scratch project ("types": ["bun-types"],strict) and ran the repository'stsc(6.0.2). Before:error TS2339: Property 'message' does not exist on type 'Promise<never>'on the first sample. The second sample already type-checked. After: no errors.Placement of the
.catch. It is after the assertions, the same as intest/js/bun/util/peek.test.ts. This order shows thatpeekleft the rejection unhandled. The samples also pass with the.catchbefore the assertions.Other shapes I tried for the first sample.
expect(peek(rejected)).toBe(error)and.toEqual(new Error(...))run correctly but do not type-check, becausetoBeandtoEqualtake the type of theexpectargument (Promise<never>).Promise.reject<Error>(...)plusas Errortype-checks and is what the test file does, but the type argument ofPromise.rejectis the fulfilled type, which misleads a reader of the docs.The
peekimplementation issrc/js/builtins/Peek.ts. It reads the promise state and the settled value through JSC intrinsics and does not touch the handled flag.prettier --check docs/runtime/utils.mdxpasses.no test proof · iteration 0 · docs-only change; test-proof not applicable