Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Apply Glia patches to Phoenix 1.6.5 #3

Open
wants to merge 6 commits into
base: v1.6.5
Choose a base branch
from
Open

Conversation

bautrey37
Copy link

@bautrey37 bautrey37 commented Jan 4, 2022

DO NOT DELETE THIS BRANCH.

This PR are to show the changes between the v1.6.5 phoenix release and the Glia patches. This PR is not meant to be merged, we will link our code to the v1.6.5-glia branch.

Phoenix versions are upgraded in:

Upgrade Phoenix to v1.6.5-glia transporter
Upgrade Phoenix to v1.6.5-glia visitor-js-api
Upgrade Phoenix to v1.6.5-glia backend-web

The reason for upgrading is to get the changes in phoenixframework#3708 which may help resolve unmatched topic errors like https://sentry.io/organizations/glia/issues/2733180193/?project=1859731. Upgrading to the latest Phoenix also has other useful changes as well.

The patches are taken from https://github.com/salemove/phoenix/commits/ca74888-glia. The patches could not be cleaned applied or were no longer applicable.

The assets/js/phoenix.js file from v1.5 was split up into multiple smaller files in v1.6. Therefore the JS patch changes had to be manually applied.

allow custom error response from socket handler phoenixframework#3821 PR implemented custom error responses, which conflicts with our solution of 4876cff. Phoenix's version did not support custom error status codes, so to maintain original functionality I overwrote our changes from Phoenix changes to ensure the same behavior, to not risk breaking anything. These custom error codes are used in Use a different error code when overloaded transporter#320, Use maximum backoff upon 503 or 429 when refreshing access token visitor-js-api#291, salemove/visitor-js-api@c89a015, and Custom error codes backend-web#5950

Webpack was removed in phoenix in replace of esbuild. Replace node+npm+webpack by esbuild phoenixframework#4377 / Switch to esbuild, package js as ESM phoenixframework#4372. Therefore this commit is not needed: 40869e3

installer/templates/phx_static/phoenix.js file no longer exists, therefore this does not need updating: 3395c2d#diff-d4085b6b2e82eef03cfec7474ad9a135ab9a86c02914984b5ad91bfd3bd2c9e3

EN-6031

indrekj and others added 4 commits January 4, 2022 16:32
Syncing presences was very slow due to using clone(). For channels that
have couple of thousand of users where users are joining/leaving and we
also have updates, clone() used almost all the CPU.

onJoin and onLeave callbacks were merged into one onChange callback.
This is because previous onJoin and onLeave callbacks were very
confusing and this way we don't have to use the the clone function.
Previously onJoin and onLeave both were called with 3 arguments: key,
currentPresence and newPresence/leftPresence. The currentPresence
however was the same value as before the update for onJoin, but updated
presence (joins/leaves processed) for onLeave. So in onJoin case we get
the state before the changes and in onLeave case we get the state after
all the changes.

Also the old onJoin newPresence variables included metas that were in
leaves payload which made it hard to detect a join vs an update.

New onChange callback includes the previous presence (state before the
updates) and new presence (state after the updates) which makes it much
easier to process changes and see what has happened.

onJoin and onLeave are still in the API because of backwards
compatibility but they're marked as deprecated.

This addresses both phoenixframework#3509 and phoenixframework#3475
Previously there were only two options, `{:ok, socket}` and `:error`. The
error always returned `403 Forbidden` to the user.

This is quite limiting. In our use case, we do rate limiting in the
connect function. E.g. if one of the phoenix servers goes down or
restarts, then we cannot immediately handle tens of thousands of users
connecting at the same time. Instead we let in some users, and let
others reconnect later. Also if we're under a heavy load then we want to
limit users connecting.

"Forbidden" is not a good status code for us. It indicates that
something is likely wrong with the access token / authorization. In that
case we want user to refresh the access token and reconnect. However if
we're under heavy load, we want to say user to try again much later.
E.g. maybe return 429 or 503 status code.

This commit allows now returning `{:error, status}` in the
Socket#connect function. It is backwards compatible: just `:error` is
still mapped to Forbidden.

This PR had to overwrite similar changes done on Phoenix to create custom
error codes: phoenixframework#3821. In
the Phoenix PR, custom error status code was not possible, only the error reason.
57e0cd9 added logic to handle 403 error codes. Now that the server can
return custom error codes, I think it makes sense to make it more
generic. The error information is passed to the `onerror` callback so
that the user can handle it as needed (e.g. reconnect immediately,
reconnect sometime in the future or maybe not reconect at all).
Ran `mix assets.build` to incorporate patches into priv files.
@bautrey37 bautrey37 changed the title V1.6.5 glia Apply Glia patches to Phoenix 1.6.2 Jan 4, 2022
@bautrey37 bautrey37 changed the title Apply Glia patches to Phoenix 1.6.2 Apply Glia patches to Phoenix 1.6.5 Jan 4, 2022
Make the code compatible with IE11 where Array.prototype.find is
undefined. This could also be achieved with a - rather complex - change
in babel configuration and pony-filling the `.find`, but this change is
just easier, with the cost of being a little harder to re-apply to
upstream changes.
Build assets using `mix assets.build`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants