fix(react-native): wait for AfterQuery subscribers before settling by kyungseopk1m · Pull Request #12767 · typeorm/typeorm

kyungseopk1m · 2026-08-08T16:33:04Z

Problem

ReactNativeQueryRunner.query() treats the two executeSql callbacks differently.

The success callback waits for the AfterQuery subscribers before resolving:

if (broadcasterResult.promises.length > 0)
    await Promise.all(broadcasterResult.promises)
...
ok(result)

The error callback broadcasts and rejects immediately:

this.broadcaster.broadcastAfterQueryEvent(broadcasterResult, query, parameters, false, undefined, undefined, err)

fail(new QueryFailedError(query, parameters, err))

So on a failed query the caller sees the rejection before the subscribers have run. A subscriber that records failures may still be in flight, and one that throws produces an unhandled rejection, since fail() has already been called and nobody is holding its promise.

The outer finally { await broadcasterResult.wait() } did not help. executeSql is callback based, so the try body returns as soon as the call is registered and the finally runs while broadcasterResult.promises is still empty. Neither callback has fired at that point. That is exactly why the success path had to await a second time inside the callback.

The success path has a related hole. If a subscriber rejects, await Promise.all(...) rejects inside an async callback with no handler, so neither ok() nor fail() is ever called and the query promise hangs forever.

Fix

Await the subscribers on both paths and drop the outer finally, which was dead.

The two paths handle a throwing subscriber differently, matching the cordova and nativescript runners:

success: the query itself was fine, so the subscriber error is the only failure there is. Reject with it.
error: there is already a real failure. A subscriber that throws while logging it should not replace the QueryFailedError the caller needs to see, so its error is swallowed.

Tests

test/unit/driver/query-broadcast-events.test.ts drives the runner against a mocked executeSql:

a failing query waits for a slow subscriber before rejecting
a failing query with a rejecting subscriber still rejects with the original error, and the subscriber rejection does not escape as an unhandledRejection
a successful query with a rejecting subscriber rejects instead of hanging

Each case is guarded by a timeout so a regression into the hang fails the test rather than stalling the run. Reverting the error path fails the first two, reverting the success path fails the third.

pnpm run test:fast is otherwise unchanged.

The success callback awaited the AfterQuery subscribers before resolving, but the error callback rejected straight after broadcasting. A subscriber that logs or transforms failures never finished before the caller saw the rejection, and a subscriber that threw surfaced as an unhandled rejection because fail() had already run. The outer finally could not cover this: executeSql is callback based, so it ran before either callback fired, with an empty promise list. That is why the success path awaited again inside the callback. Drop it. The success path now rejects with the subscriber error, while the error path swallows it so it cannot hide the original QueryFailedError. This matches how the same pair is handled in the cordova and nativescript runners.

pkg-pr-new · 2026-08-08T16:34:06Z

typeorm-sql-js-example
```
npm i https://pkg.pr.new/typeorm@12767
```

commit: 69c69b4

qodo-free-for-open-source-projects · 2026-08-08T16:35:52Z

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (1) 📜 Skill insights (0)

1. as unknown as ReactNativeDriver cast 📘 Rule violation ⚙ Maintainability

Description

The new unit test stubs bypass TypeScript typing by double-casting objects to ReactNativeDriver,
which can hide real type/shape errors in the test setup. This conflicts with the guideline to avoid
type-bypassing casts in new changes.

Code

test/unit/driver/query-broadcast-events.test.ts[87]
+            } as unknown as ReactNativeDriver

Evidence
PR Compliance ID 4 requires avoiding type-bypassing casts in new code. The added test stubs use `as
unknown as ReactNativeDriver` (and similar) to coerce objects into the driver type, which is exactly
the kind of type-bypass the rule calls out.
Rule 4: Remove AI-generated noise
test/unit/driver/query-broadcast-events.test.ts[76-88]
test/unit/driver/query-broadcast-events.test.ts[96-108]
test/unit/driver/query-broadcast-events.test.ts[127-139]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The test uses `as unknown as ReactNativeDriver` to force a stub object into the `ReactNativeDriver` type, which bypasses type checking and can mask incorrect/missing fields.

## Issue Context
This pattern appears multiple times in the new test and is discouraged by the compliance checklist.

## Fix Focus Areas
- test/unit/driver/query-broadcast-events.test.ts[76-88]
- test/unit/driver/query-broadcast-events.test.ts[96-108]
- test/unit/driver/query-broadcast-events.test.ts[127-139]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. Subscriber wait short-circuits 🐞 Bug ☼ Reliability

Description

ReactNativeQueryRunner awaits broadcasterResult.wait(), but BroadcasterResult.wait() uses
Promise.all which rejects on the first subscriber failure and returns before other afterQuery
promises finish. On the error path (where that rejection is swallowed), the query can still reject
before slower AfterQuery subscribers complete, undermining the intended ordering.

Code

src/driver/react-native/ReactNativeQueryRunner.ts[R153-156]

+                        try {
+                            await broadcasterResult.wait()
+                        } catch {
+                            // a subscriber failing must not hide the original query error

Evidence
The error callback awaits broadcasterResult.wait() and then rejects with the query error; however
BroadcasterResult.wait() uses Promise.all, which settles as soon as one promise rejects, so
other subscriber promises may still be pending when the query is rejected.
src/driver/react-native/ReactNativeQueryRunner.ts[137-160]
src/subscriber/BroadcasterResult.ts[15-24]
src/subscriber/Broadcaster.ts[499-528]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`ReactNativeQueryRunner` now awaits `broadcasterResult.wait()` before settling the query promise, but `BroadcasterResult.wait()` is implemented with `Promise.all(...)`. If any subscriber rejects, `Promise.all` rejects immediately and does **not** wait for other subscriber promises to finish. In the error callback you catch and swallow that rejection, then call `fail(new QueryFailedError(...))`, which can still settle the query promise while other (slower) AfterQuery subscribers are in flight.

## Issue Context
This is most visible with multiple `afterQuery` subscribers: one rejects quickly, another resolves slowly. The quick rejection causes `Promise.all` to reject early, and the slow subscriber continues running after the query promise has already rejected.

## Fix Focus Areas
- src/driver/react-native/ReactNativeQueryRunner.ts[103-108]
- src/driver/react-native/ReactNativeQueryRunner.ts[153-160]
- src/subscriber/BroadcasterResult.ts[16-24]

## Suggested fix
- In the **error callback**, replace `await broadcasterResult.wait()` with an implementation that waits for *all* subscriber promises to settle, e.g. `await Promise.allSettled(broadcasterResult.promises)` (or a small helper), then always reject with the original `QueryFailedError`.
- In the **success callback**, if you want the same “wait for everyone” guarantee even when a subscriber fails, use `Promise.allSettled` and after it completes reject with the subscriber error (e.g., first rejection reason) instead of failing fast.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Author self-review: I have reviewed the code review findings, and addressed the relevant ones.

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

kyungseopk1m · 2026-08-08T16:42:49Z

Both are pre-existing: BroadcasterResult.wait() is shared by every driver, and the casts match the existing test stubs.

github-actions Bot added the needs-triage PR needs issue link triage label Aug 8, 2026

qodo-free-for-open-source-projects Bot added the Compliance violation label Aug 8, 2026

github-actions Bot added linked-issue PR references an issue and removed needs-triage PR needs issue link triage labels Aug 8, 2026

github-actions Bot mentioned this pull request Aug 27, 2026

fix(react-native): await query event subscribers before settling #12808

Closed

4 tasks

Navigation Menu

Uh oh!

Uh oh!

fix(react-native): wait for AfterQuery subscribers before settling - #12767

fix(react-native): wait for AfterQuery subscribers before settling#12767
kyungseopk1m wants to merge 1 commit into
typeorm:masterfrom
kyungseopk1m:fix/react-native-await-after-query-on-error

kyungseopk1m commented Aug 8, 2026 •

edited

Loading

Uh oh!

pkg-pr-new Bot commented Aug 8, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 8, 2026

Uh oh!

kyungseopk1m commented Aug 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

1 participant

Uh oh!

Uh oh!

Conversation

kyungseopk1m commented Aug 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Fix

Tests

Related

Uh oh!

pkg-pr-new Bot commented Aug 8, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Aug 8, 2026

Code Review by Qodo

Uh oh!

kyungseopk1m commented Aug 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

1 participant

kyungseopk1m commented Aug 8, 2026 •

edited

Loading