Aborted API requests can stay in the inflight cache

[bhousel]

This code is in the OSM service, but we might have this issue in other places where we fetch data..

Rapid/modules/services/OsmService.js

Lines 366 to 375 in acdc469

	_fetch(resource, { signal: controller.signal })
	.then(utilFetchResponse)
	.then(result => gotResult(null, result))
	.catch(err => {
	if (err.name === 'AbortError') return; // ok
	if (err.name === 'FetchError') {
	gotResult(err);
	return;
	}
	});

This is unfortunately because we are mixing Promise and callback style code.
When the fetch is aborted, it returns and skips calling the errback, (amusingly the comment says // ok).
The errback is where the request would be removed from the _tileCache.inflight cache .

What we really should do is:

• Anyplace we have an inflight request cache, it should be keyed off the url+method, not the tileid.
• The fetch should have a finally block to remove the request from the inflight cache.
• While we're at it, the AbortSignal can now have a timeout. This is a thing that all modern browsers support now - details. The default fetch timeout in Chrome is 300 seconds, but we should probably drop most requests after like 5-10sec.
• (Can any requests take longer than 300 seconds? Posting a changeset? Maybe a sneaky bug lives there).

[eneerhut]

@bhousel what implications does this have for the user's experience in Rapid?

[bhousel]

what implications does this have for the user's experience in Rapid?

When the bug is triggered, it would show up as a missing square of OSM data that never loads.

[eneerhut]

I feel like I might have encountered this. Do you have a rough idea of how often this might occur?

[bhousel]

I feel like I might have encountered this. Do you have a rough idea of how often this might occur?

I'm not sure - it would probably depend on how much scrolling and zooming the user is doing.