Skip to content

Commit f171f7e

Browse files
authored
Fix false-positive sync failures (microsoft#8368)
* Fix false-positive sync failures * Add TODO to revisit
1 parent 32f42c8 commit f171f7e

File tree

1 file changed

+10
-5
lines changed

1 file changed

+10
-5
lines changed

webapp/src/cloud.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -492,7 +492,7 @@ async function syncAsyncInternal(hdrs?: Header[]): Promise<Header[]> {
492492
// Mark remote copy as deleted.
493493
pxt.debug(`Propagating ${projShorthand} delete to cloud.`)
494494
const newHdr = await toCloud(local, remoteFile.version)
495-
if (!newHdr) {
495+
if (newHdr === null) {
496496
pxt.tickEvent(`identity.sync.failed.localDeleteUpdatedCloudFailed`)
497497
throw new Error(`Failed to save ${local.id} to the cloud.`)
498498
}
@@ -515,7 +515,7 @@ async function syncAsyncInternal(hdrs?: Header[]): Promise<Header[]> {
515515
// Local has unpushed changes, push them now
516516
pxt.debug(`local project '${local.name}' has changes that will be pushed to the cloud.`)
517517
const newHdr = await toCloud(local, remoteFile.version);
518-
if (!newHdr) {
518+
if (newHdr === null) {
519519
pxt.tickEvent(`identity.sync.failed.localProjectUpdatedToCloudFailed`)
520520
throw new Error(`Failed to save ${local.id} to the cloud.`)
521521
}
@@ -549,7 +549,7 @@ async function syncAsyncInternal(hdrs?: Header[]): Promise<Header[]> {
549549
// Local cloud synced project exists, but it didn't make it to the server,
550550
// so let's push it now.
551551
const newHdr = await toCloud(local, null)
552-
if (!newHdr) {
552+
if (newHdr === null) {
553553
pxt.tickEvent(`identity.sync.failed.orphanedLocalProjectPushedToCloudFailed`)
554554
throw new Error(`Failed to save ${local.id} to the cloud.`)
555555
}
@@ -718,14 +718,19 @@ async function onHeadersChanged(): Promise<void> {
718718
const saveStart = U.nowSeconds()
719719
const saveTasks = hdrs.map(async h => {
720720
const newHdr = await transferToCloud(h, h.cloudVersion);
721-
return newHdr
721+
// If the header is deleted, return undefined. If transfer to cloud failed, return null.
722+
// This allows us to distinguish between successful sync of a deleted project, and a
723+
// sync failure.
724+
// TODO: Return a richer sync result that clearly indicates success or failure. Distinguishing
725+
// between null and undefined to tease out the error condition is not ideal, or maintainable.
726+
return h.isDeleted ? undefined : newHdr ? newHdr : null;
722727
});
723728
inProgressSavePromise = Promise.all(saveTasks);
724729

725730
// check the response
726731
try {
727732
const allRes = await inProgressSavePromise;
728-
const anyFailed = allRes.some(r => !r);
733+
const anyFailed = allRes.some(r => r === null);
729734
if (anyFailed) {
730735
pxt.tickEvent(`identity.cloudSaveFailedTriggeringPartialSync`);
731736
// if any saves failed, then we're out of sync with the cloud so resync.

0 commit comments

Comments
 (0)