contrapunctus/MapComplete
1
0
Fork 0
forked from MapComplete/MapComplete
Code Pull requests Activity

Better error handling, see #2009

Browse source
This commit is contained in:
Pieter Vander Vennet 2024-07-09 13:39:36 +02:00
parent 0db81f7b68
commit c91d691a36
2 changed files with 95 additions and 62 deletions
Download patch file Download diff file Expand all files Collapse all files

133
src/Logic/Osm/Changes.ts
View file

@ -55,7 +55,7 @@ export class Changes {
featureSwitches?: FeatureSwitchState featureSwitches?: FeatureSwitchState
}, },
leftRightSensitive: boolean = false, leftRightSensitive: boolean = false,
reportError?: (string: string | Error) => void reportError?: (string: string | Error) => void,
) { ) {
this._leftRightSensitive = leftRightSensitive this._leftRightSensitive = leftRightSensitive
// We keep track of all changes just as well // We keep track of all changes just as well
@ -70,7 +70,7 @@ export class Changes {
state.osmConnection, state.osmConnection,
state.featurePropertiesStore, state.featurePropertiesStore,
this, this,
(e) => this._reportError(e) (e) => this._reportError(e),
) )
this.historicalUserLocations = state.historicalUserLocations this.historicalUserLocations = state.historicalUserLocations
@ -84,7 +84,7 @@ export class Changes {
modifiedObjects: OsmObject[] modifiedObjects: OsmObject[]
newObjects: OsmObject[] newObjects: OsmObject[]
deletedObjects: OsmObject[] deletedObjects: OsmObject[]
} },
): string { ): string {
const changedElements = allChanges.modifiedObjects ?? [] const changedElements = allChanges.modifiedObjects ?? []
const newElements = allChanges.newObjects ?? [] const newElements = allChanges.newObjects ?? []
@ -173,7 +173,7 @@ export class Changes {
docs: "The identifier of the used background layer, this will probably be an identifier from the [editor layer index](https://github.com/osmlab/editor-layer-index)", docs: "The identifier of the used background layer, this will probably be an identifier from the [editor layer index](https://github.com/osmlab/editor-layer-index)",
}, },
], ],
"default" "default",
), ),
...addSource(ChangeTagAction.metatags, "ChangeTag"), ...addSource(ChangeTagAction.metatags, "ChangeTag"),
...addSource(ChangeLocationAction.metatags, "ChangeLocation"), ...addSource(ChangeLocationAction.metatags, "ChangeLocation"),
@ -201,7 +201,7 @@ export class Changes {
: "", : "",
]), ]),
source, source,
]) ]),
), ),
]) ])
} }
@ -250,7 +250,7 @@ export class Changes {
const changeDescriptions = await action.Perform(this) const changeDescriptions = await action.Perform(this)
changeDescriptions[0].meta.distanceToObject = this.calculateDistanceToChanges( changeDescriptions[0].meta.distanceToObject = this.calculateDistanceToChanges(
action, action,
changeDescriptions changeDescriptions,
) )
this.applyChanges(changeDescriptions) this.applyChanges(changeDescriptions)
} }
@ -264,7 +264,7 @@ export class Changes {
public CreateChangesetObjects( public CreateChangesetObjects(
changes: ChangeDescription[], changes: ChangeDescription[],
downloadedOsmObjects: OsmObject[] downloadedOsmObjects: OsmObject[],
): { ): {
newObjects: OsmObject[] newObjects: OsmObject[]
modifiedObjects: OsmObject[] modifiedObjects: OsmObject[]
@ -316,6 +316,8 @@ export class Changes {
r.members = change.changes["members"] r.members = change.changes["members"]
osmObj = r osmObj = r
break break
default:
throw "Got an invalid change.type: " + change.type
} }
if (osmObj === undefined) { if (osmObj === undefined) {
throw "Hmm? This is a bug" throw "Hmm? This is a bug"
@ -424,14 +426,14 @@ export class Changes {
result.modifiedObjects.length, result.modifiedObjects.length,
"modified;", "modified;",
result.deletedObjects, result.deletedObjects,
"deleted" "deleted",
) )
return result return result
} }
private calculateDistanceToChanges( private calculateDistanceToChanges(
change: OsmChangeAction, change: OsmChangeAction,
changeDescriptions: ChangeDescription[] changeDescriptions: ChangeDescription[],
) { ) {
const locations = this.historicalUserLocations?.features?.data const locations = this.historicalUserLocations?.features?.data
if (locations === undefined) { if (locations === undefined) {
@ -451,7 +453,7 @@ export class Changes {
.filter((feat) => feat.geometry.type === "Point") .filter((feat) => feat.geometry.type === "Point")
.filter((feat) => { .filter((feat) => {
const visitTime = new Date( const visitTime = new Date(
(<GeoLocationPointProperties>(<any>feat.properties)).date (<GeoLocationPointProperties>(<any>feat.properties)).date,
) )
// In seconds // In seconds
const diff = (now.getTime() - visitTime.getTime()) / 1000 const diff = (now.getTime() - visitTime.getTime()) / 1000
@ -498,30 +500,32 @@ export class Changes {
...recentLocationPoints.map((gpsPoint) => { ...recentLocationPoints.map((gpsPoint) => {
const otherCoor = GeoOperations.centerpointCoordinates(gpsPoint) const otherCoor = GeoOperations.centerpointCoordinates(gpsPoint)
return GeoOperations.distanceBetween(coor, otherCoor) return GeoOperations.distanceBetween(coor, otherCoor)
}) }),
) ),
) ),
) )
} }
/** /**
* Upload the selected changes to OSM. * Gets a single, fresh version of the requested osmObject with some error handling
* Returns 'true' if successful and if they can be removed
*/ */
private async flushSelectChanges( private async getOsmObject(id: string, downloader: OsmObjectDownloader) {
pending: ChangeDescription[],
openChangeset: UIEventSource<number>
): Promise<boolean> {
const self = this
const neededIds = Changes.GetNeededIds(pending)
// We _do not_ pass in the Changes object itself - we want the data from OSM directly in order to apply the changes
const downloader = new OsmObjectDownloader(this.backend, undefined)
let osmObjects = await Promise.all<{ id: string; osmObj: OsmObject | "deleted" }>(
neededIds.map(async (id) => {
try { try {
try {
// Important: we do **not** cache this request, we _always_ need a fresh version! // Important: we do **not** cache this request, we _always_ need a fresh version!
const osmObj = await downloader.DownloadObjectAsync(id, 0) const osmObj = await downloader.DownloadObjectAsync(id, 0)
return { id, osmObj } return { id, osmObj }
} catch (e) {
const msg = "Could not download OSM-object" +
id +
" trying again before dropping it from the changes (" +
e +
")"
this._reportError(msg)
const osmObj = await downloader.DownloadObjectAsync(id, 0)
return { id, osmObj }
}
} catch (e) { } catch (e) {
const msg = "Could not download OSM-object" + const msg = "Could not download OSM-object" +
id + id +
@ -533,7 +537,22 @@ export class Changes {
this.errors.ping() this.errors.ping()
return undefined return undefined
} }
}) }
/**
* Upload the selected changes to OSM. This is typically called with changes for a single theme
* @return pending changes which could not be uploaded for some reason; undefined or empty array if successful
*/
private async flushSelectChanges(
pending: ChangeDescription[],
openChangeset: UIEventSource<number>,
): Promise<ChangeDescription[]> {
const self = this
const neededIds = Changes.GetNeededIds(pending)
// We _do not_ pass in the Changes object itself - we want the data from OSM directly in order to apply the changes
const downloader = new OsmObjectDownloader(this.backend, undefined)
let osmObjects = await Promise.all<{ id: string; osmObj: OsmObject | "deleted" }>(
neededIds.map(id => this.getOsmObject(id, downloader)),
) )
osmObjects = Utils.NoNull(osmObjects) osmObjects = Utils.NoNull(osmObjects)
@ -554,7 +573,7 @@ export class Changes {
if (pending.length == 0) { if (pending.length == 0) {
console.log("No pending changes...") console.log("No pending changes...")
return true return undefined
} }
const perType = Array.from( const perType = Array.from(
@ -562,15 +581,15 @@ export class Changes {
pending pending
.filter( .filter(
(descr) => (descr) =>
descr.meta.changeType !== undefined && descr.meta.changeType !== null descr.meta.changeType !== undefined && descr.meta.changeType !== null,
) )
.map((descr) => descr.meta.changeType) .map((descr) => descr.meta.changeType),
), ),
([key, count]) => ({ ([key, count]) => ({
key: key, key: key,
value: count, value: count,
aggregate: true, aggregate: true,
}) }),
) )
const motivations = pending const motivations = pending
.filter((descr) => descr.meta.specialMotivation !== undefined) .filter((descr) => descr.meta.specialMotivation !== undefined)
@ -609,7 +628,7 @@ export class Changes {
value: count, value: count,
aggregate: true, aggregate: true,
} }
}) }),
) )
// This method is only called with changedescriptions for this theme // This method is only called with changedescriptions for this theme
@ -633,26 +652,42 @@ export class Changes {
...perBinMessage, ...perBinMessage,
] ]
const refused: ChangeDescription[] = []
let toUpload: ChangeDescription[] = []
pending.forEach(c => {
if (c.id < 0) {
toUpload.push(c)
return
}
const matchFound = !!objects.find(o => o.id === c.id && o.type === c.type)
if (matchFound) {
toUpload.push(c)
} else {
refused.push(c)
}
})
await this._changesetHandler.UploadChangeset( await this._changesetHandler.UploadChangeset(
(csId, remappings) => { (csId, remappings) => {
if (remappings.size > 0) { if (remappings.size > 0) {
pending = pending.map((ch) => ChangeDescriptionTools.rewriteIds(ch, remappings)) toUpload = toUpload.map((ch) => ChangeDescriptionTools.rewriteIds(ch, remappings))
} }
const changes: { const changes: {
newObjects: OsmObject[] newObjects: OsmObject[]
modifiedObjects: OsmObject[] modifiedObjects: OsmObject[]
deletedObjects: OsmObject[] deletedObjects: OsmObject[]
} = self.CreateChangesetObjects(pending, objects) } = self.CreateChangesetObjects(toUpload, objects)
return Changes.createChangesetFor("" + csId, changes) return Changes.createChangesetFor("" + csId, changes)
}, },
metatags, metatags,
openChangeset openChangeset,
) )
console.log("Upload successful!") console.log("Upload successful! Refused changes are",refused)
return true return refused
} }
private async flushChangesAsync(): Promise<void> { private async flushChangesAsync(): Promise<void> {
@ -670,44 +705,42 @@ export class Changes {
pendingPerTheme.get(theme).push(changeDescription) pendingPerTheme.get(theme).push(changeDescription)
} }
const successes = await Promise.all( const refusedChanges: ChangeDescription[][] = await Promise.all(
Array.from(pendingPerTheme, async ([theme, pendingChanges]) => { Array.from(pendingPerTheme, async ([theme, pendingChanges]) => {
try { try {
const openChangeset = UIEventSource.asInt( const openChangeset = UIEventSource.asInt(
this.state.osmConnection.GetPreference( this.state.osmConnection.GetPreference(
"current-open-changeset-" + theme "current-open-changeset-" + theme,
) ),
) )
console.log( console.log(
"Using current-open-changeset-" + "Using current-open-changeset-" +
theme + theme +
" from the preferences, got " + " from the preferences, got " +
openChangeset.data openChangeset.data,
) )
const result = await self.flushSelectChanges(pendingChanges, openChangeset) const refused = await self.flushSelectChanges(pendingChanges, openChangeset)
if (result) { if (!refused) {
this.errors.setData([]) this.errors.setData([])
} }
return result return refused
} catch (e) { } catch (e) {
this._reportError(e) this._reportError(e)
console.error("Could not upload some changes:", e) console.error("Could not upload some changes:", e)
this.errors.data.push(e) this.errors.data.push(e)
this.errors.ping() this.errors.ping()
return false return pendingChanges
} }
}) }),
) )
if (!successes.some((s) => s == false)) { // We keep all the refused changes to try them again
// All changes successfull, we clear the data! this.pendingChanges.setData(refusedChanges.flatMap(c => c))
this.pendingChanges.setData([])
}
} catch (e) { } catch (e) {
console.error( console.error(
"Could not handle changes - probably an old, pending changeset in localstorage with an invalid format; erasing those", "Could not handle changes - probably an old, pending changeset in localstorage with an invalid format; erasing those",
e e,
) )
this.errors.data.push(e) this.errors.data.push(e)
this.errors.ping() this.errors.ping()

4
src/Logic/Osm/ChangesetHandler.ts
View file

@ -154,7 +154,7 @@ export class ChangesetHandler {
if (this._reportError) { if (this._reportError) {
this._reportError(e) this._reportError(e)
} }
console.warn("Could not open/upload changeset due to ", e, "trying t") console.warn("Could not open/upload changeset due to ", e, "trying again with a another fresh changeset ")
openChangeset.setData(undefined) openChangeset.setData(undefined)
throw e throw e
@ -187,7 +187,7 @@ export class ChangesetHandler {
await this.UpdateTags(csId, rewrittenTags) await this.UpdateTags(csId, rewrittenTags)
} catch (e) { } catch (e) {
if (this._reportError) { if (this._reportError) {
this._reportError(e) this._reportError("Could not reuse changeset, might be closed: " + e.stacktrace ?? "" + e)
} }
console.warn("Could not upload, changeset is probably closed: ", e) console.warn("Could not upload, changeset is probably closed: ", e)
openChangeset.setData(undefined) openChangeset.setData(undefined)