"noconnection" console errors on page navigation in maybePushComponentsDestroyed

[dushyantss]

Environment

• Elixir version (elixir -v): Elixir 1.18.3, Erlang 27.3.3
• Phoenix version (mix deps): 1.7.21
• Phoenix LiveView version (mix deps): 1.0.10
• Operating system: Both MacOS and Linux(in Github CI)
• Browsers you attempted to reproduce this bug on (the more the merrier): Safari, Chrome, Electron(Cypress tests)
• Does the problem persist after removing "assets/node_modules" and trying again? Yes/no: Yes

Actual behavior

In assets/js/phoenix_live_view/view.js's maybePushComponentsDestroyed method, we call pushWithReply. pushWithReply raises the noconnection error if the channel is no longer connected.

The issue occurs when the parent View is destroyed(in my case, due to navigation) after maybePushComponentsDestroyed is called and before one of the 2 pushWithReplys within it are called. This raises the noconnection error.

I would've loved to provide a reproducible file, but I've not been successful with it yet. I hope the explanation is enough, otherwise I'll try again to create the file where we can reliably reproduce the issue.

Expected behavior

We should not even try to pushWithReply if the channel is already closed. So, there should be no such error raised.

Suggested solution

  maybePushComponentsDestroyed(destroyedCIDs){
    let willDestroyCIDs = destroyedCIDs.filter(cid => {
      return DOM.findComponentNodeList(this.el, cid).length === 0
    })

    if(willDestroyCIDs.length > 0){
      // we must reset the render change tracking for cids that
      // could be added back from the server so we don't skip them
      willDestroyCIDs.forEach(cid => this.rendered.resetRender(cid))

      if(!this.isDestroyed()){ // <-------------- new addition
        this.pushWithReply(null, "cids_will_destroy", {cids: willDestroyCIDs}).then(() => {
          // we must wait for pending transitions to complete before determining
          // if the cids were added back to the DOM in the meantime (#3139)
          this.liveSocket.requestDOMUpdate(() => {
            // See if any of the cids we wanted to destroy were added back,
            // if they were added back, we don't actually destroy them.
            let completelyDestroyCIDs = willDestroyCIDs.filter(cid => {
              return DOM.findComponentNodeList(this.el, cid).length === 0
            })

            if(!this.isDestroyed() && completelyDestroyCIDs.length > 0){ // <---------------- the isDestroyed check is a new addition
              this.pushWithReply(null, "cids_destroyed", {cids: completelyDestroyCIDs}).then(({resp}) => {
                this.rendered.pruneCIDs(resp.cids)
              }).catch((error) => logError("Failed to push components destroyed", error))
            }
          })
        }).catch((error) => logError("Failed to push components destroyed", error))
      }
    }
  }

The only thing is that I've not yet gone through the entirety of the js code so there might be a better approach.

How I came upon it

In our CI we use Cypress to run tests with cypress-fail-on-console-error plugin. Since #3766, we're seeing the error {error: 'noconnection'} in the logs which is consistently failing one of our specs.

[SteffenDE]

Hey @dushyantss,

I opened up #3797 to silence those logs. This could happen for other events as well, so maybe we want to have a more generic solution in the future, but for now it should be good enough, I think.