zulip/static/js/fetch_status.js

137 lines
5.2 KiB
JavaScript
Raw Normal View History

import * as message_scroll from "./message_scroll";
function max_id_for_messages(messages) {
let max_id = 0;
for (const msg of messages) {
max_id = Math.max(max_id, msg.id);
}
return max_id;
}
export class FetchStatus {
message view: Fetch again when "newest" is discarded. The previous commit introduced a bug where it was not intuitive for the user to scroll again. For the current narrow, new messages were fetched again only when scrolled to the bottom as usually there are many messages displayed. However when the edge case mentioned in the previous commit occured, it was not very obvious that a scroll should be done or we could already be at the bottom and could not scroll again to trigger a fetch. `message_viewport.at_bottom` has a relevant comment explaining this behaviour. The previous commit handled the rare race condition. However, there is a possibility that the rare race condition might occur again while we are handling the previous condition. This commit resolves these 2 problems by performing a re-fetch while also resetting the `expected_max_message_id` and this approach has two benefits: 1. The reset prevents an infinite loop, if somehow the expected max message's id gets corrupted resulting in a situation where the server can never send an id greater than that even after fetching. 2. Even though we stop after just one re-fetch the race condition might recursively occur while we handle the previous race condition. And even though the reset prevents multiple re-fetches, we don't have the missing message problem. This is because we treat the next race condition as a new race condition instead of it being a continuation of the previous. The `expected_max_message_id` gets updated again, on receiving a new message. Thus it can again enter the `fetch_status` block as the reset value is updated again.
2020-06-16 17:58:37 +02:00
// The FetchStatus object tracks tracks the state of a
// message_list_data object, whether rendered in the DOM or not,
// and is the source of truth for whether the message_list_data
// object has the complete history of the view or whether more
// messages should be loaded when scrolling to the top or bottom
// of the message feed.
_loading_older = false;
_loading_newer = false;
_found_oldest = false;
_found_newest = false;
_history_limited = false;
message view: Fetch again when "newest" is discarded. The previous commit introduced a bug where it was not intuitive for the user to scroll again. For the current narrow, new messages were fetched again only when scrolled to the bottom as usually there are many messages displayed. However when the edge case mentioned in the previous commit occured, it was not very obvious that a scroll should be done or we could already be at the bottom and could not scroll again to trigger a fetch. `message_viewport.at_bottom` has a relevant comment explaining this behaviour. The previous commit handled the rare race condition. However, there is a possibility that the rare race condition might occur again while we are handling the previous condition. This commit resolves these 2 problems by performing a re-fetch while also resetting the `expected_max_message_id` and this approach has two benefits: 1. The reset prevents an infinite loop, if somehow the expected max message's id gets corrupted resulting in a situation where the server can never send an id greater than that even after fetching. 2. Even though we stop after just one re-fetch the race condition might recursively occur while we handle the previous race condition. And even though the reset prevents multiple re-fetches, we don't have the missing message problem. This is because we treat the next race condition as a new race condition instead of it being a continuation of the previous. The `expected_max_message_id` gets updated again, on receiving a new message. Thus it can again enter the `fetch_status` block as the reset value is updated again.
2020-06-16 17:58:37 +02:00
// Tracks the highest message ID that we know exist in this view,
// but are not within the contiguous range of messages we have
// received from the server. Used to correctly handle a rare race
// condition where a newly sent message races with fetching a
// group of messages that would lead to found_newest being set
// (described in detail below).
_expected_max_message_id = 0;
start_older_batch(opts) {
this._loading_older = true;
if (opts.update_loading_indicator) {
message_scroll.show_loading_older();
}
}
finish_older_batch(opts) {
this._loading_older = false;
this._found_oldest = opts.found_oldest;
this._history_limited = opts.history_limited;
if (opts.update_loading_indicator) {
message_scroll.hide_loading_older();
}
}
can_load_older_messages() {
return !this._loading_older && !this._found_oldest;
}
has_found_oldest() {
return this._found_oldest;
}
history_limited() {
return this._history_limited;
}
start_newer_batch(opts) {
this._loading_newer = true;
if (opts.update_loading_indicator) {
message_scroll.show_loading_newer();
}
}
finish_newer_batch(messages, opts) {
message view: Fetch again when "newest" is discarded. The previous commit introduced a bug where it was not intuitive for the user to scroll again. For the current narrow, new messages were fetched again only when scrolled to the bottom as usually there are many messages displayed. However when the edge case mentioned in the previous commit occured, it was not very obvious that a scroll should be done or we could already be at the bottom and could not scroll again to trigger a fetch. `message_viewport.at_bottom` has a relevant comment explaining this behaviour. The previous commit handled the rare race condition. However, there is a possibility that the rare race condition might occur again while we are handling the previous condition. This commit resolves these 2 problems by performing a re-fetch while also resetting the `expected_max_message_id` and this approach has two benefits: 1. The reset prevents an infinite loop, if somehow the expected max message's id gets corrupted resulting in a situation where the server can never send an id greater than that even after fetching. 2. Even though we stop after just one re-fetch the race condition might recursively occur while we handle the previous race condition. And even though the reset prevents multiple re-fetches, we don't have the missing message problem. This is because we treat the next race condition as a new race condition instead of it being a continuation of the previous. The `expected_max_message_id` gets updated again, on receiving a new message. Thus it can again enter the `fetch_status` block as the reset value is updated again.
2020-06-16 17:58:37 +02:00
// Returns true if and only if the caller needs to trigger an
// additional fetch due to the race described below.
message list: Render new messages only after "newest" is found. If a user sends a message while the latest batch of messages are being fetched, the new message recieved from `server_events` gets displayed temporarily out of order (just after the the current batch of messages) for the current narrow. We could just discard the new message events if we havent recieved the last message i.e. when `found_newest` = False, since we would recieve them on furthur fetching of that narrow. But this would create another bug where the new messages sent while fetching the last batch of messages would not get rendered. Because, `found_newest` = True and we would no longer fetch messages for that narrow, thus the new messages would not get fetched and are also discarded from the events codepath. Thus to resolve both these bugs we use the following approach: * We do not add the new batch of messages for the current narrow while `has_found_newest` = False. * We store the latest message id which should be displayed at the bottom of the narrow in `fetch_status`. * Ideally `expected_max_message_id`'s value should be equal to the last item's id in `MessageListData`. * So the messages received while `has_found_newest` = False, will be fetched later and also the `expected_max_message_id` value gets updated. * And after fetching the last batch where `has_found_newest` = True, we would again fetch messages if the `expected_max_message_id` is greater than the last message's id found on fetching by refusing to update the server provided `has_found_newest` = True in `fetch_status`. Another benefit of not discarding the events is that the message gets processed not rendered i.e. we still get desktop notifications and unread count updates. Fixes #14017
2020-05-30 17:34:07 +02:00
const found_max_message_id = max_id_for_messages(messages);
this._loading_newer = false;
this._found_newest = opts.found_newest;
if (opts.update_loading_indicator) {
message_scroll.hide_loading_newer();
}
if (this._found_newest && this._expected_max_message_id > found_max_message_id) {
message view: Fetch again when "newest" is discarded. The previous commit introduced a bug where it was not intuitive for the user to scroll again. For the current narrow, new messages were fetched again only when scrolled to the bottom as usually there are many messages displayed. However when the edge case mentioned in the previous commit occured, it was not very obvious that a scroll should be done or we could already be at the bottom and could not scroll again to trigger a fetch. `message_viewport.at_bottom` has a relevant comment explaining this behaviour. The previous commit handled the rare race condition. However, there is a possibility that the rare race condition might occur again while we are handling the previous condition. This commit resolves these 2 problems by performing a re-fetch while also resetting the `expected_max_message_id` and this approach has two benefits: 1. The reset prevents an infinite loop, if somehow the expected max message's id gets corrupted resulting in a situation where the server can never send an id greater than that even after fetching. 2. Even though we stop after just one re-fetch the race condition might recursively occur while we handle the previous race condition. And even though the reset prevents multiple re-fetches, we don't have the missing message problem. This is because we treat the next race condition as a new race condition instead of it being a continuation of the previous. The `expected_max_message_id` gets updated again, on receiving a new message. Thus it can again enter the `fetch_status` block as the reset value is updated again.
2020-06-16 17:58:37 +02:00
// This expected_max_message_id logic is designed to
// resolve a subtle race condition involving newly sent
// messages in a view that does not display the currently
// latest messages.
//
// When a new message arrives matching the current view
// and found_newest is false, we cannot add the message to
// the view in-order without creating invalid output
// (where two messages are displaye adjacent but might be
// weeks and hundreds of messages apart in actuality).
//
// So we have to discard those messages. Usually, this is
// fine; the client will receive those when the user
// scrolls to the bottom of the page, triggering another
// fetch. With that solution, a rare race is still possible,
// with this sequence:
//
// 1. Client initiates GET /messages to fetch the last
// batch of messages in this view. The server
// completes the database access and and starts sending
// the response with found_newest=true.
// 1. A new message is sent matching the view, the event reaches
// the client. We discard the message because found_newest=false.
// 1. The client receives the GET /messages response, and
// marks found_newest=true. As a result, it believes is has
// the latest messages and won't fetch more, but is missing the
// recently sent message.
//
// To address this problem, we track the highest message
// ID among messages that were discarded due to
// fetch_status in expected_max_message_id. If that is
// higher than the highest ID returned in a GET /messages
// response with found_newest=true, we know the above race
// has happened and trigger an additional fetch.
this._found_newest = false;
message view: Fetch again when "newest" is discarded. The previous commit introduced a bug where it was not intuitive for the user to scroll again. For the current narrow, new messages were fetched again only when scrolled to the bottom as usually there are many messages displayed. However when the edge case mentioned in the previous commit occured, it was not very obvious that a scroll should be done or we could already be at the bottom and could not scroll again to trigger a fetch. `message_viewport.at_bottom` has a relevant comment explaining this behaviour. The previous commit handled the rare race condition. However, there is a possibility that the rare race condition might occur again while we are handling the previous condition. This commit resolves these 2 problems by performing a re-fetch while also resetting the `expected_max_message_id` and this approach has two benefits: 1. The reset prevents an infinite loop, if somehow the expected max message's id gets corrupted resulting in a situation where the server can never send an id greater than that even after fetching. 2. Even though we stop after just one re-fetch the race condition might recursively occur while we handle the previous race condition. And even though the reset prevents multiple re-fetches, we don't have the missing message problem. This is because we treat the next race condition as a new race condition instead of it being a continuation of the previous. The `expected_max_message_id` gets updated again, on receiving a new message. Thus it can again enter the `fetch_status` block as the reset value is updated again.
2020-06-16 17:58:37 +02:00
// Resetting our tracked last message id is an important
// circuit-breaker for cases where the message(s) that we
// "know" exist were deleted or moved to another topic.
this._expected_max_message_id = 0;
message view: Fetch again when "newest" is discarded. The previous commit introduced a bug where it was not intuitive for the user to scroll again. For the current narrow, new messages were fetched again only when scrolled to the bottom as usually there are many messages displayed. However when the edge case mentioned in the previous commit occured, it was not very obvious that a scroll should be done or we could already be at the bottom and could not scroll again to trigger a fetch. `message_viewport.at_bottom` has a relevant comment explaining this behaviour. The previous commit handled the rare race condition. However, there is a possibility that the rare race condition might occur again while we are handling the previous condition. This commit resolves these 2 problems by performing a re-fetch while also resetting the `expected_max_message_id` and this approach has two benefits: 1. The reset prevents an infinite loop, if somehow the expected max message's id gets corrupted resulting in a situation where the server can never send an id greater than that even after fetching. 2. Even though we stop after just one re-fetch the race condition might recursively occur while we handle the previous race condition. And even though the reset prevents multiple re-fetches, we don't have the missing message problem. This is because we treat the next race condition as a new race condition instead of it being a continuation of the previous. The `expected_max_message_id` gets updated again, on receiving a new message. Thus it can again enter the `fetch_status` block as the reset value is updated again.
2020-06-16 17:58:37 +02:00
return true;
}
return false;
}
can_load_newer_messages() {
return !this._loading_newer && !this._found_newest;
}
2018-05-03 19:44:11 +02:00
has_found_newest() {
return this._found_newest;
}
message list: Render new messages only after "newest" is found. If a user sends a message while the latest batch of messages are being fetched, the new message recieved from `server_events` gets displayed temporarily out of order (just after the the current batch of messages) for the current narrow. We could just discard the new message events if we havent recieved the last message i.e. when `found_newest` = False, since we would recieve them on furthur fetching of that narrow. But this would create another bug where the new messages sent while fetching the last batch of messages would not get rendered. Because, `found_newest` = True and we would no longer fetch messages for that narrow, thus the new messages would not get fetched and are also discarded from the events codepath. Thus to resolve both these bugs we use the following approach: * We do not add the new batch of messages for the current narrow while `has_found_newest` = False. * We store the latest message id which should be displayed at the bottom of the narrow in `fetch_status`. * Ideally `expected_max_message_id`'s value should be equal to the last item's id in `MessageListData`. * So the messages received while `has_found_newest` = False, will be fetched later and also the `expected_max_message_id` value gets updated. * And after fetching the last batch where `has_found_newest` = True, we would again fetch messages if the `expected_max_message_id` is greater than the last message's id found on fetching by refusing to update the server provided `has_found_newest` = True in `fetch_status`. Another benefit of not discarding the events is that the message gets processed not rendered i.e. we still get desktop notifications and unread count updates. Fixes #14017
2020-05-30 17:34:07 +02:00
update_expected_max_message_id(messages) {
this._expected_max_message_id = Math.max(
this._expected_max_message_id,
max_id_for_messages(messages),
);
}
}