mirror of
https://github.com/stablyai/orca.git
synced 2026-09-30 00:03:15 +00:00
fix(mobile-web): retry the package download across a logical-session cutover
A relay/direct cutover rejects in-flight requests without changing `connState`. The capability probe retries; the package downloader did not. Every throw collapsed to `host_error`, and the refresh effect's deps are all unchanged by a seamless cutover, so nothing re-ran and the user had to tap Retry — on the one screen that has no content yet. The contract now marks a cutover or ambiguous-delivery throw `retryable`, using the same two predicates the native-chat send path already trusts, and both package reads re-issue it on the replacement session with bounded backoff: the chunk read next to its existing read-limited retry, and the manifest read, which is the one request a cutover can kill before any chunk exists to retry. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
This commit is contained in:
@@ -21,6 +21,8 @@ import {
|
||||
|
||||
const MOBILE_WEB_PACKAGE_READ_LIMITED_RETRIES = 4
|
||||
const MOBILE_WEB_PACKAGE_READ_LIMITED_BACKOFF_MS = 50
|
||||
const MOBILE_WEB_PACKAGE_CUTOVER_RETRIES = 4
|
||||
const MOBILE_WEB_PACKAGE_CUTOVER_BACKOFF_MS = 250
|
||||
|
||||
type ChunkTask = { asset: MobileWebAsset; offset: number; expectedLength: number }
|
||||
type SettledChunk = { bytes: Uint8Array } | { failure: unknown }
|
||||
@@ -162,6 +164,16 @@ async function fetchChunk<TCommit>(
|
||||
await sleep(MOBILE_WEB_PACKAGE_READ_LIMITED_BACKOFF_MS * (attempt + 1))
|
||||
continue
|
||||
}
|
||||
// Why: a seamless cutover replaces the logical session without changing connState, so the
|
||||
// request is dead but the download is not; the user should not have to tap Retry.
|
||||
if (
|
||||
error instanceof MobileWebPackageDownloadError &&
|
||||
error.retryable &&
|
||||
attempt < MOBILE_WEB_PACKAGE_CUTOVER_RETRIES
|
||||
) {
|
||||
await sleep(MOBILE_WEB_PACKAGE_CUTOVER_BACKOFF_MS * (attempt + 1))
|
||||
continue
|
||||
}
|
||||
return { failure: error }
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
import { isRpcDeliveryUnknown } from '../transport/rpc-delivery-ambiguity'
|
||||
import { isLogicalClientCutoverError } from '../transport/stable-logical-rpc-client'
|
||||
import {
|
||||
isMobileWebPackageErrorCode,
|
||||
type MobileWebPackageErrorCode
|
||||
@@ -27,7 +29,11 @@ export type MobileWebPackageDownloadErrorCode =
|
||||
| MobileWebPackageErrorCode
|
||||
|
||||
export class MobileWebPackageDownloadError extends Error {
|
||||
constructor(readonly code: MobileWebPackageDownloadErrorCode) {
|
||||
constructor(
|
||||
readonly code: MobileWebPackageDownloadErrorCode,
|
||||
/** The same request on a fresh logical session can still succeed. */
|
||||
readonly retryable = false
|
||||
) {
|
||||
super(code)
|
||||
this.name = 'MobileWebPackageDownloadError'
|
||||
}
|
||||
@@ -55,8 +61,13 @@ export async function requestMobileWebPackageResult(
|
||||
let response: RpcResponse
|
||||
try {
|
||||
response = await request(method, params)
|
||||
} catch {
|
||||
throw new MobileWebPackageDownloadError('host_error')
|
||||
} catch (error) {
|
||||
// Why: a relay/direct cutover rejects in-flight requests without changing connState, so no
|
||||
// upstream effect re-runs the download. Mark it so the caller can re-issue on the new session.
|
||||
throw new MobileWebPackageDownloadError(
|
||||
'host_error',
|
||||
isRpcDeliveryUnknown(error) || isLogicalClientCutoverError(error)
|
||||
)
|
||||
}
|
||||
if (!response.ok) {
|
||||
const message = response.error.message
|
||||
|
||||
@@ -259,6 +259,28 @@ describe('mobile web package downloader', () => {
|
||||
downloadMobileWebPackage(request, stager, { shellBridgeVersion: 1 })
|
||||
).rejects.toEqual(new MobileWebPackageDownloadError('host_method_unavailable'))
|
||||
})
|
||||
// A relay/direct cutover rejects in-flight requests without changing connState, so no effect
|
||||
// upstream re-runs the download: without a retry here the user is left tapping Retry by hand.
|
||||
it('rides out a logical-session cutover on the manifest read and on a chunk read', async () => {
|
||||
const fixture = createFixture({ manifestCutoverFailures: 1, chunkCutoverFailures: 1 })
|
||||
const stager = createStager()
|
||||
|
||||
const result = await downloadMobileWebPackage(fixture.request, stager, {
|
||||
shellBridgeVersion: 1
|
||||
})
|
||||
|
||||
expect(result.manifest).toEqual(fixture.manifest)
|
||||
expect(stager.commit).toHaveBeenCalledOnce()
|
||||
expect(stager.abort).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('still fails when the cutovers outlast the bounded retry', async () => {
|
||||
const fixture = createFixture({ manifestCutoverFailures: 99 })
|
||||
|
||||
await expect(
|
||||
downloadMobileWebPackage(fixture.request, createStager(), { shellBridgeVersion: 1 })
|
||||
).rejects.toMatchObject({ code: 'host_error' })
|
||||
})
|
||||
})
|
||||
|
||||
function createFixture(
|
||||
@@ -271,6 +293,8 @@ function createFixture(
|
||||
gzipOversizedOutput?: boolean
|
||||
invalidBuildIdentity?: boolean
|
||||
afterFirstChunk?: () => void
|
||||
manifestCutoverFailures?: number
|
||||
chunkCutoverFailures?: number
|
||||
} = {}
|
||||
): Fixture {
|
||||
const document = Buffer.from('<!doctype html><title>Orca</title>')
|
||||
@@ -299,8 +323,14 @@ function createFixture(
|
||||
[assets.find((candidate) => candidate.role === 'script')!.path, script]
|
||||
])
|
||||
let chunkCount = 0
|
||||
let manifestCutovers = 0
|
||||
let chunkCutovers = 0
|
||||
const request = vi.fn(async (method: string, params?: unknown): Promise<RpcResponse> => {
|
||||
if (method === 'mobileWeb.package.manifest') {
|
||||
if (manifestCutovers < (options.manifestCutoverFailures ?? 0)) {
|
||||
manifestCutovers += 1
|
||||
throw new Error('RPC interrupted by connection migration')
|
||||
}
|
||||
return success({
|
||||
manifest: options.invalidBuildIdentity
|
||||
? { ...manifest, buildId: 'f'.repeat(64) }
|
||||
@@ -308,6 +338,10 @@ function createFixture(
|
||||
chunkBytes: MOBILE_WEB_PACKAGE_CHUNK_BYTES
|
||||
})
|
||||
}
|
||||
if (chunkCutovers < (options.chunkCutoverFailures ?? 0)) {
|
||||
chunkCutovers += 1
|
||||
throw new Error('RPC interrupted by connection migration')
|
||||
}
|
||||
const assetParams = params as { buildId: string; path: string; offset: number }
|
||||
const bytes = bytesByPath.get(assetParams.path)!
|
||||
const chunk = bytes.subarray(
|
||||
|
||||
@@ -17,6 +17,9 @@ import {
|
||||
type MobileWebPackageStager
|
||||
} from './mobile-web-package-download-contract'
|
||||
|
||||
const MOBILE_WEB_PACKAGE_MANIFEST_CUTOVER_RETRIES = 4
|
||||
const MOBILE_WEB_PACKAGE_MANIFEST_CUTOVER_BACKOFF_MS = 250
|
||||
|
||||
export {
|
||||
MOBILE_WEB_PACKAGE_DOWNLOAD_ERROR_CODES,
|
||||
MobileWebPackageDownloadError,
|
||||
@@ -82,10 +85,7 @@ export async function downloadMobileWebPackage<TCommit>(
|
||||
}
|
||||
): Promise<ReusedOrDownloadedMobileWebPackage<TCommit>> {
|
||||
throwIfAborted(options.signal)
|
||||
const manifestResponse = await requestMobileWebPackageResult(
|
||||
request,
|
||||
'mobileWeb.package.manifest'
|
||||
)
|
||||
const manifestResponse = await readManifestAcrossCutovers(request, options.signal)
|
||||
throwIfAborted(options.signal)
|
||||
const parsedManifest = MobileWebPackageManifestResponseSchema.safeParse(manifestResponse)
|
||||
if (!parsedManifest.success) {
|
||||
@@ -167,3 +167,28 @@ function throwIfAborted(signal: AbortSignal | undefined): void {
|
||||
throw new MobileWebPackageDownloadError('cancelled')
|
||||
}
|
||||
}
|
||||
|
||||
// Why: the manifest read is the one request a cutover can kill before any chunk exists to retry,
|
||||
// and nothing upstream re-runs the download when connState never changed.
|
||||
async function readManifestAcrossCutovers(
|
||||
request: MobileWebPackageRequest,
|
||||
signal: AbortSignal | undefined
|
||||
): Promise<unknown> {
|
||||
for (let attempt = 0; ; attempt += 1) {
|
||||
try {
|
||||
return await requestMobileWebPackageResult(request, 'mobileWeb.package.manifest')
|
||||
} catch (error) {
|
||||
if (
|
||||
!(error instanceof MobileWebPackageDownloadError) ||
|
||||
!error.retryable ||
|
||||
attempt >= MOBILE_WEB_PACKAGE_MANIFEST_CUTOVER_RETRIES
|
||||
) {
|
||||
throw error
|
||||
}
|
||||
await new Promise((resolve) =>
|
||||
setTimeout(resolve, MOBILE_WEB_PACKAGE_MANIFEST_CUTOVER_BACKOFF_MS * (attempt + 1))
|
||||
)
|
||||
throwIfAborted(signal)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user