diff --git a/src/domain/session/room/timeline/tiles/GapTile.js b/src/domain/session/room/timeline/tiles/GapTile.js index 1e6bdd08..66e83147 100644 --- a/src/domain/session/room/timeline/tiles/GapTile.js +++ b/src/domain/session/room/timeline/tiles/GapTile.js @@ -24,71 +24,59 @@ export class GapTile extends SimpleTile { constructor(entry, options) { super(entry, options); this._loading = false; - this._error = null; + this._waitingForConnection = false; this._isAtTop = true; this._siblingChanged = false; - this._showSpinner = false; } get needsDateSeparator() { return false; } - async fill() { + async fill(isRetrying = false) { if (!this._loading && !this._entry.edgeReached) { this._loading = true; - this._error = null; - this._showSpinner = true; this.emitChange("isLoading"); try { await this._room.fillGap(this._entry, 10); } catch (err) { - console.error(`room.fillGap(): ${err.message}:\n${err.stack}`); - this._error = err; if (err instanceof ConnectionError) { - this.emitChange("error"); - /* - We need to wait for reconnection here rather than in - notifyVisible() because when we return/throw here - this._loading is set to false and other queued invocations of - this method will succeed and attempt further room.fillGap() calls - - resulting in multiple error entries in logs and elsewhere! - */ await this._waitForReconnection(); + if (!isRetrying) { + // retry after the connection comes back + // if this wasn't already a retry after coming back online + return await this.fill(true); + } else { + return false; + } + } else { + this.reportError(err); + return false; } - // rethrow so caller of this method - // knows not to keep calling this for now - throw err; } finally { this._loading = false; - this._showSpinner = false; this.emitChange("isLoading"); } - return true; + return true; } return false; } async notifyVisible() { + // if any error happened before (apart from being offline), + // let the user dismiss the error before trying to backfill + // again so we don't try to do backfill the don't succeed + // in quick succession + if (this.errorViewModel) { + return; + } // we do (up to 10) backfills while no new tiles have been added to the timeline // because notifyVisible won't be called again until something gets added to the timeline let depth = 0; let canFillMore; this._siblingChanged = false; do { - try { - canFillMore = await this.fill(); - } - catch (e) { - if (e instanceof ConnectionError) { - canFillMore = true; - // Don't increase depth because this gap fill was a noop - continue; - } - else { - canFillMore = false; - } - } + canFillMore = await this.fill(); depth = depth + 1; } while (depth < 10 && !this._siblingChanged && canFillMore && !this.isDisposed); } @@ -124,7 +112,11 @@ export class GapTile extends SimpleTile { } async _waitForReconnection() { + this._waitingForConnection = true; + this.emitUpdate("status"); await this.options.client.reconnector.connectionStatus.waitFor(status => status === ConnectionStatus.Online).promise; + this._waitingForConnection = false; + this.emitUpdate("status"); } get shape() { @@ -136,29 +128,19 @@ export class GapTile extends SimpleTile { } get showSpinner() { - return this._showSpinner; + return this.isLoading || this._waitingForConnection; } - get error() { - if (this._error) { - if (this._error instanceof ConnectionError) { - return "Waiting for reconnection"; - } - const dir = this._entry.prev_batch ? "previous" : "next"; - return `Could not load ${dir} messages: ${this._error.message}`; - } - return null; - } - - get currentAction() { - if (this.error) { - return this.error; - } - else if (this.isLoading) { - return "Loading"; - } - else { - return "Not Loading"; + get status() { + const dir = this._entry.prev_batch ? "previous" : "next"; + if (this._waitingForConnection) { + return "Waiting for connection…"; + } else if (this.errorViewModel) { + return `Could not load ${dir} messages`; + } else if (this.isLoading) { + return "Loading more messages…"; + } else { + return "Gave up loading more messages"; } } } diff --git a/src/matrix/room/BaseRoom.js b/src/matrix/room/BaseRoom.js index 7eb73faf..4ea25389 100644 --- a/src/matrix/room/BaseRoom.js +++ b/src/matrix/room/BaseRoom.js @@ -350,7 +350,7 @@ export class BaseRoom extends EventEmitter { fragmentIdComparer: this._fragmentIdComparer, relationWriter }); - gapResult = await gapWriter.writeFragmentFill(fragmentEntry, response, txn, log); + gapResult = await gapWriter.writeFragmentFill(fragmentEntry, response, fragmentEntry.token, txn, log); } catch (err) { txn.abort(); throw err; diff --git a/src/matrix/room/timeline/persistence/GapWriter.js b/src/matrix/room/timeline/persistence/GapWriter.js index 4458e1c5..d9ad5476 100644 --- a/src/matrix/room/timeline/persistence/GapWriter.js +++ b/src/matrix/room/timeline/persistence/GapWriter.js @@ -154,10 +154,13 @@ export class GapWriter { return changedFragments; } - async writeFragmentFill(fragmentEntry, response, txn, log) { + /** + * @param {string} fromToken the token used to call /messages, to ensure it hasn't changed in storage + */ + async writeFragmentFill(fragmentEntry, response, fromToken, txn, log) { const {fragmentId, direction} = fragmentEntry; // chunk is in reverse-chronological order when backwards - const {chunk, start, state} = response; + const {chunk, state} = response; let {end} = response; if (!Array.isArray(chunk)) { @@ -174,8 +177,8 @@ export class GapWriter { } fragmentEntry = fragmentEntry.withUpdatedFragment(fragment); // check that the request was done with the token we are aware of (extra care to avoid timeline corruption) - if (fragmentEntry.token !== start) { - throw new Error("start is not equal to prev_batch or next_batch"); + if (fragmentEntry.token !== fromToken) { + throw new Error("The pagination token has changed locally while fetching messages."); } // begin (or end) of timeline reached @@ -263,7 +266,7 @@ export function tests() { async function backfillAndWrite(mocks, fragmentEntry, limit) { const {txn, timelineMock, gapWriter} = mocks; const messageResponse = timelineMock.messages(fragmentEntry.token, undefined, fragmentEntry.direction.asApiString(), limit); - await gapWriter.writeFragmentFill(fragmentEntry, messageResponse, txn, logger); + await gapWriter.writeFragmentFill(fragmentEntry, messageResponse, fragmentEntry.token, txn, logger); } async function allFragmentEvents(mocks, fragmentId) { diff --git a/src/platform/web/ui/css/themes/element/error.css b/src/platform/web/ui/css/themes/element/error.css index ce252056..e0b945ca 100644 --- a/src/platform/web/ui/css/themes/element/error.css +++ b/src/platform/web/ui/css/themes/element/error.css @@ -4,9 +4,14 @@ margin: 16px; } -.ErrorView_inline { +.ErrorView.ErrorView_inline { color: var(--error-color); - margin: 4px; + margin: 4px 0; + padding: 4px 0; +} + +.ErrorView.ErrorView_inline > p { + margin: 0; } .ErrorView { @@ -48,4 +53,4 @@ .ErrorView_inline .ErrorView_close { background-image: url('icons/clear.svg?primary=text-color'); -} \ No newline at end of file +} diff --git a/src/platform/web/ui/css/themes/element/theme.css b/src/platform/web/ui/css/themes/element/theme.css index fd8e69b0..4c617386 100644 --- a/src/platform/web/ui/css/themes/element/theme.css +++ b/src/platform/web/ui/css/themes/element/theme.css @@ -769,6 +769,11 @@ a { margin: 0; } + +.FeatureView p { + margin: 8px 0; +} + .error { color: var(--error-color); font-weight: 600; diff --git a/src/platform/web/ui/css/themes/element/timeline.css b/src/platform/web/ui/css/themes/element/timeline.css index 8606400d..4a822605 100644 --- a/src/platform/web/ui/css/themes/element/timeline.css +++ b/src/platform/web/ui/css/themes/element/timeline.css @@ -411,7 +411,7 @@ only loads when the top comes into view*/ border-radius: 10px; } -.GapView > :not(:first-child) { +.GapView_container > :not(:first-child) { margin-left: 12px; } diff --git a/src/platform/web/ui/css/timeline.css b/src/platform/web/ui/css/timeline.css index b6d62c98..0932739c 100644 --- a/src/platform/web/ui/css/timeline.css +++ b/src/platform/web/ui/css/timeline.css @@ -52,11 +52,11 @@ limitations under the License. align-items: center; } -.GapView { +.GapView_container { display: flex; } -.GapView > :nth-child(2) { +.GapView_container > span { flex: 1; } diff --git a/src/platform/web/ui/session/room/timeline/GapView.js b/src/platform/web/ui/session/room/timeline/GapView.js index 4fc0e3d6..91181c09 100644 --- a/src/platform/web/ui/session/room/timeline/GapView.js +++ b/src/platform/web/ui/session/room/timeline/GapView.js @@ -16,6 +16,7 @@ limitations under the License. import {TemplateView} from "../../../general/TemplateView"; import {spinner} from "../../../common.js"; +import {ErrorView} from "../../../general/ErrorView"; export class GapView extends TemplateView { // ignore other argument @@ -23,15 +24,20 @@ export class GapView extends TemplateView { super(vm); } - render(t) { + render(t, vm) { const className = { GapView: true, isLoading: vm => vm.isLoading, isAtTop: vm => vm.isAtTop, }; return t.li({ className }, [ - t.if(vm => vm.showSpinner, (t) => spinner(t)), - t.span(vm => vm.currentAction) + t.div({class: "GapView_container"}, [ + t.if(vm => vm.showSpinner, (t) => spinner(t)), + t.span(vm => vm.status), + ]), + t.if(vm => !!vm.errorViewModel, t => { + return t.view(new ErrorView(vm.errorViewModel, {inline: true})); + }) ]); }