Instead of prohibiting ‘return undefined’ (#8669), we require that a
function must return an explicit value always or never. This prevents
you from forgetting to return a value in some cases. It will also be
important for TypeScript, which distinguishes between undefined and
void.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
ES and TypeScript modules are strict by default and don’t need this
directive. ESLint will remind us to add it to new CommonJS files and
remove it from ES and TypeScript modules.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Prettier would do this anyway, but it’s separated out for a more
reviewable diff. Generated by ESLint.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Prettier would do this anyway, but it’s separated out for a more
reviewable diff. Generated by ESLint.
Signed-off-by: Anders Kaseorg <anders@zulip.com>
Since we are no longer using the "pointer" value sent in
page_params.pointer for anything, there's no value in continuing to
send it from the server to the client.
The remaining code in pointer.js is logic managing state for the
currently selected message.
This is not always a behavior-preserving translation: _.extend mutates
its first argument. However, the code does not always appear to have
been written to expect that.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
This generalizes existing code for the presence code path that is
generically useful for avoiding useless work that will be discarded.
We make an exception for the one type of request that needs to happen
while reloading, namely the one to clean up our event queue.
This should return us to a situation where we won't get blueslip
browser error reporting for users created while a device was offline
just before it reloads.
The _.each calls with an inline function expression have already been
converted to for…of loops. We could do that here, but using .forEach
when we’re just reusing an existing function seems like a good
guideline.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
We now use user_ids for presence, so we don't need
to worry about races related to unknown emails
being sent to us. Now we just update the data
structure based on user_id, and
it will be there when we render the presence
widget for that user_id, or else it will
simply be ignored.
It's not clear to me whether we still need
dont_block here, so I didn't touch that code.
Here is the commit that added the suspect_offline
flag, for easy reference:
f207450cdb
This flag affects page_params and the
payload you get back from POSTs to this
url:
users/me/presence
The flag does not yet affect the
presence events that get sent to a
client.
This commit was originally automatically generated using `tools/lint
--only=eslint --fix`. It was then modified by tabbott to contain only
changes to a set of files that are unlikely to result in significant
merge conflicts with any open pull request, excluding about 20 files.
His plan is to merge the remaining changes with more precise care,
potentially involving merging parts of conflicting pull requests
before running the `eslint --fix` operation.
Signed-off-by: Anders Kaseorg <anders@zulipchat.com>
With webpack, variables declared in each file are already file-local
(Global variables need to be explicitly exported), so these IIFEs are
no longer needed.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
After migration to an ES6 module, `server_furthest_read` would no
longer be mutable from outside the module.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
When we're handling a single message that was locally echoed, there
will very likely be 0 messages not removed by
`echo.process_from_server`, and we can skip the unnecessary call to
`message_events.insert_new_messages`. This is a small performance
optimization and logical simplification when sending messages.
Show "sent to different narrow" notification and other such notification by
notifications.notify_local_mixes for non locally echoed message sent by
current client.
With significant new comments added by tabbott.
Fixes: #11488.
Apparently, we didn't have one of these, and thus had a moderate
number of generally very old violations in the codebase. Fix this and
clear the ones that exist..
When there is some error in connecting to server(more specifically to the
tornado server) the "Unable to connect to Zulip" connection error message
gets cleared as Django server could send the response of "get" request of
old messages and hence get_old_messages_success hides the error message
even though the connection is not properly established.
Fixes: #5599.
This is general fix that makes sure that we
apply all message-modifying events after we
apply the events for the initial incoming
messages.
The particular scenario that was reported here
was when you would have two tabs for Zulip,
with one of them open and in a PM view, and
with the open tab being at the bottom of the
feed, such that incoming messages would be
immediately visible.
Now suppose the other person in that PM
conversation sent you a message.
The open tab would properly immediately
mark the message as read, and notify
the server. The problem was that the closed
tab would not process the main message event
until it "woke up", by which time the flag-update
event was bundled into the same event batch
as the main message event. We'd then process
the flag-update first, which essentially was
a noop, since the actual message wasn't in
the message store yet. The user would then
see unread counts increment in the closed tab,
while the open tab didn't increment. This
was confusing.
Now `server_events.js` processes the actual
message first and does the flag-update as part of a
`post_message_events` loop.
We include events for updating message flags,
deleting messages, and attaching submessages
to messages in the `post_message_events` array.
This bug was a bit difficult to simulate in a dev
environment, since you needed your "open" tab
to be in focus to simulate the race, but as
soon as you tab to another place to deliver
a message (whether from the browser or otherwise),
the open tab is no longer in focus.
I did this in the console of my "open"
tab to work around it:
unread_ops.process_visible = unread_ops.mark_current_list_as_read;
This problem was easy to reproduce, but it wasn't
entirely consistent. I often needed to send
several messages in succession to trigger event
batching and force the race condition. (This wasn't
precisely a "race", as events actually arrive in the
correct order; it was having them arrive in the same
batch that triggered the bug.)
This is part of work to break some of our
nastier circular dependencies in preparation
for our es6 migration.
This commit should facilitate loading leaf-like
modules such as people.js before all of the things
that reload.js depends on.
This commit prepares the frontend code to be consumed by webpack.
It is a hack: In theory, modules should be declaring and importing the
modules they depend on and the globals they expose directly.
However, that requires significant per-module work, which we don't
really want to block moving our toolchain to webpack on.
So we expose the modules by setting window.varName = varName; as
needed in the js files.
We no longer set message.flags in the local echo path.
In the markdown parsing step, we just set message.mentioned
directly.
And then we change `insert_new_messages` to no longer
convert flags to booleans, and move that code to only
happen for incoming server message events.
We sometimes get blueslip errors from browsers that are clearly still
attempting to reload long after they should have. These browsers can
produce a lot of unnecessary presence update exceptions.
To solve that, we start checking reload_in_progress in the presence
code path.
While we're at it, we also add some blueslip logging for the reload
code path, in case it becomes useful when debugging future issues.
The issue has a lot of extra details, but in short, if several
messages were sent at very close to the same time, it's possible that
the event queues will receive the "new message" events out-of-order.
This, in turn, could cause `get_events` to return an incorrectly
sorted block of messages. These would then be passed into
`message_list.add_messages`, which doesn't handle that sort of
unsorted situation correctly (in short, the `self.first.id()`
comparison checks are not accurate for that situation, since we don't
update the boundaries after the first messages is processed).
The end result of this bug was that it was possible for the message
list to be out-of-order, which in turn would cause exceptions when
scrolling with the mouse.
Fixes#6948.
This commit extract send_messages.js to clean up code related
to the following things:
* sending data to /json/report_send_time
* restarting the event loop if events don't arrive on time
The code related to /json/report changes the following ways:
* We track the state almost completely in the new
send_messages.js module, with other modules just
making one-line calls.
* We no longer send "displayed" times to the servers, since
we were kind of lying about them anyway.
* We now explicitly track the state of each single sent
message in its own object.
* We now look up data related to the messages by local_id,
instead of message_id. The problem with message_id was
that is was mutable. Now we use local_id, and we extend
the local_id concept to messages that don't get rendered
client side. We no longer need to react to the
'message_id_changed' event to change our hash key.
* The code used to live in many places:
* various big chunks were scattered among compose.js,
and those were all moved or reduced to one-line
calls into the new module
* echo.js continues to make basically one-line calls,
but it no longer calls compose.report_as_received(),
nor does it set the "start" time.
* message_util.js used to report received events, but
only when they finally got drawn in the home view;
this code is gone now
The code related to restarting the event loop if events don't arrive
changes as follows:
* The timer now gets set up from within
send_messages.message_state.report_server_ack,
where we can easily inspect the current state of the
possibly-still-in-flight message.
* The code to confirm that an event was received happens now
in server_events.js, rather than later, so that we don't
falsely blame the event loop for a downstream bug. (Plus
it's easier to just do it one place.)
This change removes a fair amount of code from our node tests. Some
of the removal is good stuff related to us completing killing off
unnecessary code. Other removals are more expediency-driven, and
we should make another sweep at ramping up our coverage on compose.js,
with possibly a little more mocking of the new `send_messages` code
layer, since it's now abstracted better.
There is also some minor cleanup to echo.resend_message() in this
commit.
See #5968 for a detailed breakdown of the changes.
This fixes the original issue that #5598 was the root cause of; when
the user returns to a Zulip browser tab after they've been idle past
the timeout (10 min, per IDLE_EVENT_QUEUE_TIMEOUT_SECS), we now
correctly reload the page even if they're using Zulip in German or
another non-English language where we have a translation for the
relevant error message.
I pushed a bunch of commits that attempted to introduce
the concept of `client_message_id` into our server, as
part of cleaning up our codepaths related to messages you
sent (both for the locally echoed case and for the host
case).
When we deployed this, we had some strange failures involving
double-echoed messages and issues advancing the pointer that appeared
related to #5779. We didn't get to the bottom of exactly why the PR
caused havoc, but I decided there was a cleaner approach, anyway.
We now use a client-side message id to track the state of our
sent messages. This sets up future commits to start tracking
state earlier in the message's life cycle.
It also avoids ugly reify logic where we capture an event to
update our data structure to key on the server's message id
instead of the local id. That eliminates the node test as well.
Another node test gets deleted here, just because it's not
worth the trouble with upcoming refactorings.
This commit starts to decouple client_message_id from local_id.
We don't really take advantage of the decoupling in this
commit--in fact, it's a bit of a pain at first. But this should
be a fully working checkpoint commit.
We are deprecating local_id/local_message_id on the Python server.
Instead of the server knowing about the client's implementation of
local id, with the message id = 9999.01 scheme, we just send the
server an opaque id to send back to us.
This commit changes the name from local_id -> client_message_id,
but it doesn't change the actual values passed yet.
The goal for client_key in future commits will be to:
* Have it for all messages, not just locally rendered messages
* Not have it overlap with server-side message ids.
The history behind local_id having numbers like 9999.01 is that
they are actually interim message ids and the numerical value is
used for rendering the message list when we do client-side rendering.
Anders Kaseorg
Aman Agrawal
Tim Abbott
Anders Kaseorg
Tim Abbott
Steve Howell
Anders Kaseorg
Mohit Gupta
Shubham Dhama
Armaan Ahluwalia
Rishi Gupta
Steve Howell
Greg Price
Cory Lynch