Skip to content

  • Projects
  • Groups
  • Snippets
  • Help
  • Sign in / Register
Minds Frontend
Minds Frontend
  • Project overview
  • Repository
  • Issues 347
  • Merge Requests 56
  • CI / CD
  • Security & Compliance
  • Packages
  • Wiki
  • Snippets
  • Members
  • Collapse sidebar
  • Graph
  • Charts
  • Create a new issue
  • Jobs
  • Commits
  • Issue Boards
  • Minds
  • Minds FrontendMinds Frontend
  • Merge Requests
  • !692

Open
Opened 1 month ago by Ben Hayward@benhayward.ben
Report abuse

[Sprint/TrendyTurtle](feat): Added in error message on socket connection breaking. #2115

  • Overview 11
  • Commits 6
  • Pipelines 5
  • Changes 5
1/1 thread resolved

Summary

Closes #2115 MR to add an error message in to show the user when their connection to live comments has been lost.

Steps to test

This can be tested pretty simply.

  1. Make a post or go to visit one https://feat-socket-error-comments-2115.minds.io/newsfeed/1049089523444617221
  2. Keep the page open, drag the tab into its own window so you can watch it whilst you do step 3.
  3. On the pipeline of this MR. Run the stop job. Do not refresh the page.
  4. Wait and the disconnect message will show (because the stop job killed the sockets container)
  5. Don't close the window!
  6. Start the job again.
  7. The message should go away when the connection is re-established.

image

Estimated Regression Scope

The worst-case scenario is are that something could break in sockets or thread comments. E.g. the error showing at inappropriate times if the error is for some reason called more times than necessary on prod, or maybe the load more of the comments thread.

Edited 1 month ago by Ben Hayward
Request to merge feat/socket-error-comments-2115 into master
The source branch is 22 commits behind the target branch
Open in Web IDE
Pipeline #104735140 passed with warnings for 76c427ad on feat/socket-error-comments-2115
              Requires approval from QA. Approved by
              Brian Hatchet
              Brian Hatchet
              Rami Albatal
              Rami Albatal
              Marcelo Rivera
              Marcelo Rivera
              Guy Thouret
              Guy Thouret
              Olivia Madrid
              Olivia Madrid
              Juan Manuel Solaro
              Juan Manuel Solaro
              There are merge conflicts. Resolve these conflicts or ask someone with write access to this repository to merge it locally

              Closes #2115

              Deletes source branch

              You can merge this merge request manually using the
              • Ben Hayward @benhayward.ben added Squad::Yellow scoped label 1 month ago

                added scoped label

              • Ben Hayward @benhayward.ben added 1 commit 1 month ago

                added 1 commit

                • 5d87644a - Handle errors from CommentsService

                Compare with previous version

              • Ben Hayward @benhayward.ben added 1 commit 1 month ago

                added 1 commit

                • 41448c5b - Unused imports

                Compare with previous version

              • Ben Hayward @benhayward.ben added Status::Requires Changes scoped label 1 month ago

                added scoped label

              • Ben Hayward @benhayward.ben changed the description 1 month ago

                changed the description

              • Ben Hayward @benhayward.ben added Status::Awaiting Review scoped label and automatically removed Status::Requires Changes label 1 month ago

                added scoped label and automatically removed label

              • Ben Hayward
                Ben Hayward @benhayward.ben · 1 month ago
                Developer

                Just to highlight, this would not be particularly difficult to implement in other areas of the site.

              • Brian Hatchet :speech_balloon: @brianhatchet approved this merge request 1 month ago

                approved this merge request

              • Brian Hatchet
                Brian Hatchet :speech_balloon: @brianhatchet · 1 month ago
                Developer

                Error messaging looks great. Fantastic steps to recreate, Ben.

              • Brian Hatchet :speech_balloon: @brianhatchet assigned to @xander-miller 1 month ago

                assigned to @xander-miller

              • Brian Hatchet :speech_balloon: @brianhatchet assigned to @edgebal 1 month ago

                assigned to @edgebal

              • Xander Miller
                Xander Miller @xander-miller · 3 weeks ago
                Developer

                I need help replicating this @benhayward.ben When I stopped the pod through gitlab. I couldn't start it up again.

              • Mark Harding
                Mark Harding @markeharding started a thread on an old version of the diff 3 weeks ago
                Resolved by Ben Hayward 3 weeks ago
              • Ben Hayward
                Ben Hayward @benhayward.ben · 3 weeks ago
                Developer

                @xander-miller I'll be happy to help but the pipelines appear to be up and working now. Drop me a message if you run into this issue again.

              • Ben Hayward @benhayward.ben assigned to @benhayward.ben and unassigned @edgebal and @xander-miller 3 weeks ago

                assigned to @benhayward.ben and unassigned @edgebal and @xander-miller

              • Ben Hayward @benhayward.ben added Status::Requires Changes scoped label and automatically removed Status::Awaiting Review label 3 weeks ago

                added scoped label and automatically removed label

              • Ben Hayward
                Ben Hayward @benhayward.ben · 3 weeks ago
                Developer

                Spoke with Mark about this.

                This should also cover a "false connection" state that can be simulated by deleting cookies when on the site. The message should also reload the comments, to get any ones missed during the outage, as the socket messages are not in a LIFO queue as I'd originally anticipated.

              • Ben Hayward @benhayward.ben added 38 commits 3 weeks ago

                added 38 commits

                • 41448c5b...ea067e71 - 36 commits from branch master
                • ff3bf7f9 - Merge branch 'master' of gitlab.com:minds/front into feat/socket-error-comments-2115
                • 72d2d7c5 - Added in retry logic

                Compare with previous version

              • Ben Hayward @benhayward.ben resolved all threads 3 weeks ago

                resolved all threads

              • Ben Hayward
                Ben Hayward @benhayward.ben · 3 weeks ago
                Developer

                I could not replicate the false connection state by clearing my local storage, but did add the reconnect feature in.

              • Ben Hayward @benhayward.ben added 1 commit 3 weeks ago

                added 1 commit

                • 76c427ad - Removed debug log

                Compare with previous version

              • Ben Hayward
                Ben Hayward @benhayward.ben · 45 minutes ago
                Developer

                @brianhatchet @markeharding can we merge this in and try to find the false connection state in another card? This change is stagnating.

              • Brian Hatchet
                Brian Hatchet :speech_balloon: @brianhatchet · 37 minutes ago
                Developer

                Fine by me.

              • Brian Hatchet :speech_balloon: @brianhatchet approved this merge request 37 minutes ago

                approved this merge request

              Please register or sign in to reply
              Assignee
              Ben Hayward's avatar
              Ben Hayward @benhayward.ben
              None
              Milestone
              None
              Time tracking
              No estimate or time spent
              2
              Labels
              Squad::Yellow Status::Requires Changes
              Lock merge request
              Unlocked
              11
              11 participants
              user avatar
              Rami Albatal
              user avatar
              Marcelo Rivera
              user avatar
              Guy Thouret
              user avatar
              Olivia Madrid
              user avatar
              Juan Manuel Solaro
              user avatar
              Martin Santangelo
              user avatar
              Xander Miller
              Reference: minds/front!692

              Now you can access the merge request navigation tabs at the top, where they’re easier to find.

              More information and share feedback