bugfix: unhandled exception in disconnect function

[izinin]

bugfix: unhandled exception in disconnect function when chat panel is not shown by UI

Thanks for making a pull request to converse.js!

Before submitting your request, please make sure the following conditions are met:

• Add a changelog entry for your change in CHANGES.md
• When adding a configuration variable, please make sure to
  document it in docs/source/configuration.rst
• Please add a test for your change. Tests can be run in the commandline
  with make check or you can run them in the browser by running make serve
  and then opening http://localhost:8000/tests.html.

[jcbrand]

What is the error/exception that you're seeing?

If it's due to view not being defined, then you can do:

view?.model.set({ connected: false });

[izinin]

i need something to return in promise because it is async operation in you fix variant i will return null which will cause blowing somewhere else. That is why there is no ? operator

…

On Mon, 8 Jul 2024 at 16:04, JC Brand ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/plugins/controlbox/utils.js <#3435 (comment)> : > @@ -26,9 +26,12 @@ export function disconnect () { * we reconnect, "onConnected" will be called, * to fetch the roster again and to send out a presence stanza. */ - const view = _converse.state.chatboxviews.get('controlbox'); - view.model.set({ 'connected': false }); - return view; + try{ + const view = _converse.state.chatboxviews.get('controlbox'); + view.model.set({ 'connected': false }); What is the error/exception that you're seeing? If it's due to view not being defined, then you can do: view?.model.set({ connected: false }); — Reply to this email directly, view it on GitHub <#3435 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAT2PMCQIANBDZAZRFGXB5LZLKE5HAVCNFSM6AAAAABKNEWUWSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDCNRTGI2DSMZZHA> . You are receiving this because you authored the thread.Message ID: ***@***.***>

-- Regards, Igor

[jcbrand]

i need something to return in promise because it is async operation in you fix variant i will return null which will cause blowing somewhere else. That is why there is no ? operator

I would still like to know what the actual error is that you're getting, so that I can better understand the underlying issue.

Simply wrapping a statement with a blanket try/catch is usually not the best solution.

[izinin]

this is timing issue , _converse.state.chatboxviews might not be created yet when a view is requested.
Due to asynchronous the application nature and not constant network conditions this issue is not 100% reproducible but does happen. When there is unhandled exception in ConverseJS chat becomes blank. This issue considerable impacts users experience.
We need to use defensive programming in such conditions assuming any function in the run path can fail. We either propagate error up to the call stack or silently return. This is the second case

Also , please kindly reject the PR if you consider it not to go to the mainline

[jcbrand]

I've merged your commit manually, but then made a change, because I don't think blanket try/catch is usually the right way to go. Also, it doesn't look like the function needs to return anything.