Skip to content

Suppress JS upload updates after failures#1556

Merged
TorstenDittmann merged 1 commit into
masterfrom
fix/concurrent-upload-review-feedback
May 21, 2026
Merged

Suppress JS upload updates after failures#1556
TorstenDittmann merged 1 commit into
masterfrom
fix/concurrent-upload-review-feedback

Conversation

@TorstenDittmann
Copy link
Copy Markdown
Contributor

@TorstenDittmann TorstenDittmann commented May 21, 2026

What

  • Stop in-flight JS upload workers from mutating shared upload state or firing progress callbacks after another chunk has already failed
  • Tighten generated TypeScript ping return types from Promise to Promise
  • Applies to Node, Web/Console, and React Native templates

Why

Greptile flagged remaining low-confidence cases in downstream Node and Console SDK PRs: after the first chunk failure, already in-flight chunks could still complete their post-await code path and emit progress updates after the caller had observed a rejection.

Verification

  • composer lint-twig
  • git diff --check
  • composer test tests/Node18Test.php
  • composer test tests/WebChromiumTest.php
  • composer test tests/ReactNativeTest.php
  • composer test tests/CLIBun13Test.php

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 21, 2026

Greptile Summary

This PR suppresses stale state mutations and spurious onProgress callbacks that could fire from in-flight upload chunks after a sibling chunk had already failed. It also narrows the ping() return type from Promise<any> to Promise<unknown> across all three JS/TS SDK templates.

  • Upload guard (Node, React Native, Web): failed/rejected is moved up before the uploadChunk closure so the flag is in scope, and an early-return is inserted immediately after this.call() resolves — preventing completedCount, uploadedBytes, lastResponse, and onProgress from being mutated once another worker has already set the failure flag.
  • Type tightening: ping() return type narrowed from Promise<any> to Promise<unknown> in Node, Web, and React Native client templates, enforcing explicit downstream type narrowing instead of silently widening to any.

Confidence Score: 4/5

Safe to merge; the guard logic is correct and the only nit is a cosmetic naming inconsistency between the web and node/react-native templates.

The upload-failure guard is logically sound across all three templates. In the worker-pool pattern (Node/React Native), while (!failed && ...) already prevents new chunks from being picked up, and the new post-await check closes the window where an already-in-flight chunk could still mutate shared state. In the web callback-dispatcher pattern, moving rejected out of the Promise executor and into the outer closure is the necessary precondition for the same guard to work inside uploadChunk.

No files require special attention; the three upload-logic files received the same structural change and are internally consistent.

Important Files Changed

Filename Overview
templates/node/src/client.ts.twig Moves failed flag before uploadChunk closure (both InputFile and File variants) and adds an early-return guard after each this.call() resolves; also tightens ping() return type to Promise<unknown>.
templates/web/src/client.ts.twig Moves rejected flag from inside the Promise executor to the outer closure scope (so uploadChunk can read it) and adds the same early-return guard post-this.call(); ping() return type tightened to Promise<unknown>.
templates/react-native/src/services/template.ts.twig Same worker-pool guard as the Node template: failed moved before uploadChunk definition and early-return added after this.client.call() resolves.
templates/react-native/src/client.ts.twig Single-line change: ping() return type narrowed from Promise<any> to Promise<unknown>. No upload-logic changes.

Reviews (1): Last reviewed commit: "Suppress JS upload updates after failure" | Re-trigger Greptile

Comment thread templates/web/src/client.ts.twig
@TorstenDittmann TorstenDittmann merged commit 6397e68 into master May 21, 2026
56 checks passed
@TorstenDittmann TorstenDittmann deleted the fix/concurrent-upload-review-feedback branch May 21, 2026 17:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant