emacs-bug-tracker
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#59976: closed (ERC 5.4.1: erc-networks--id gets clobbered in erc ser


From: GNU bug Tracking System
Subject: bug#59976: closed (ERC 5.4.1: erc-networks--id gets clobbered in erc server buffer on /query name conflict)
Date: Fri, 16 Dec 2022 15:18:02 +0000

Your message dated Fri, 16 Dec 2022 07:17:18 -0800
with message-id <87r0wzgyyp.fsf@neverwas.me>
and subject line Re: bug#59976: ERC 5.4.1: erc-networks--id gets clobbered in 
erc server buffer on /query name conflict
has caused the debbugs.gnu.org bug report #59976,
regarding ERC 5.4.1: erc-networks--id gets clobbered in erc server buffer on 
/query name conflict
to be marked as done.

(If you believe you have received this mail in error, please contact
help-debbugs@gnu.org.)


-- 
59976: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=59976
GNU Bug Tracking System
Contact help-debbugs@gnu.org with problems
--- Begin Message --- Subject: ERC 5.4.1: erc-networks--id gets clobbered in erc server buffer on /query name conflict Date: Mon, 12 Dec 2022 00:00:31 +0500
Hi.

I've needed to use case-sensitive channel names (with matrix2051 ad-hoc
ircd), wanted to test updated ERC client for that, and stumbled upon
what looks like a bug there:

  When IRC server is named "slack", and then it sends you a message from
  user named "slack", ERC tries to open query-buffer "slack", ends up
  running erc--open-target("slack") which does kill-all-local-variables()
  when enabling erc-mode in the buffer.

  That removes erc-networks--id value there and from then on ERC keeps
  spamming "error in process filter" about erc-networks--id being nil.


- What I'm trying to do:

  - (erc-tls ...)  ;; cleates "slack" server buffer and some channels.
  - Open "slack" buffer
  - Send "/ping myuser" IRC-command there


- Expected result:

  Command actually results in PRIVMSG from "slack" user on this
  specific ircd, so I expect to see that open a separate query-buffer
  where that message is displayed.

  (message in question - though probably doesn't matter:
  "msg: ^APING 1670781447.080567^A could not be sent channel_not_found")


- Actual result:

  - Emacs slows down while printing following errors to MiniBuffer and 
*Messages*:

      error in process filter: or: Wrong type argument: erc-networks--id, nil
      error in process filter: Wrong type argument: erc-networks--id, nil

  - As far as I can tell that IRC connection becomes unusable, and
    errors like above get signaled from then on randomly, likely on
    some commands sent from ircd.


- Workaround: changing announced name of the server to something else
  helps - "slack" query-buffer gets created and is separate from server buffer.


Was able to understand what seem to be the issue here by enabling
logging for erc-networks--id changes to *Messages* like this:

  (defun debug-watch-log (sym newval op where)
    (message "Variable update: %s = %S -> %S [%S %S]"
      sym (symbol-value sym) newval op where)
    (backtrace))
  (add-variable-watcher 'erc-networks--id #'debug-watch-log)

(run M-x cancel-debug-on-variable-change afterwards to disable this)


I think some kind of disambiguation or conflict-detection for
query-channel names might be either missing or not working atm,
but is needed to avoid this happening unintentionally, or maybe
even on purpose (e.g. to annoy someone by cutting their IRC connection).

I'm running ERC 5.4.1 from current 0e5d059 git master on Emacs 28.2
(replacing "erc" dir in /usr/share/emacs), so it is also possible that
maybe some change in Emacs 29+ prevents this behavior, but it seems unlikely.


Thanks.

-- 
Mike Kazantsev // fraggod.net



--- End Message ---
--- Begin Message --- Subject: Re: bug#59976: ERC 5.4.1: erc-networks--id gets clobbered in erc server buffer on /query name conflict Date: Fri, 16 Dec 2022 07:17:18 -0800 User-agent: Gnus/5.13 (Gnus v5.13)
Mike Kazantsev <mk.fraggod@gmail.com> writes:

> On Thu, 15 Dec 2022 06:16:08 -0800
> "J.P." <jp@neverwas.me> wrote:
>
>> Anyhow, I have attempted to address this in the attached patch. If you
>> or anyone out there is willing, please install it locally atop the
>> current lisp/erc subtree on the emacs-29 branch and see if you can break
>> it. Thanks in advance!
>
> I think you indeed found the same thing I was thinking of and fixed it, 
> thanks.
>
> You mentioned connection process and how ID is being set in the previous
> message, which indeed didn't seem to be same issues as I've tried to describe
> (not clearly enough).

Well, I've long been in the terrible habit of just saying "connection"
when I really mean "logical IRC connection" and not the actual network
process. But sometimes I mean both or one then the other, which is
obviously insane but likely what happened here (so please just take
whatever I said with a grain of salt).

> So thought to make a quick mockup of an IRC server and then send it,
> with precise commands to run from ERC to reproduce exact problem,
> as well actual output from that variable-change-tracking (which shows
> how/when ID gets erased after successful connection, and only after
> triggering conflicting query-buffer creation).
>
> Haven't gotten around to do it yet though, but pretty sure you found
> and fixed same exact issue in 0001-Fix-some-naming-issues-*.patch here,
> so there should no longer be any need to do it.
>
> I've just tested removing my custom ID-setting advices/code, using :id
> instead, as well as reproducing that exact query-buffer-name conflict,
> and it all works without any issues that I can see.
> So will use that instead of local code hacks and won't need to worry
> about making server-buffer names unique/distinct.

Nice!

> One small thing I've noticed about :id is that it has separate "Network
> Identifier" section in "Connecting" info page, which doesn't seem to be linked
> directly in its description - not sure if intentional, or maybe an oversight:
>
>   When present, ID should be an opaque object for identifying the
>   connection unequivocally.  (In most cases, this would be a string or a
>   symbol composed of letters from the Latin alphabet.)  This option is
>   generally unneeded, however.  See info node ‘(erc) Connecting’ for use
>   cases.  Not available interactively.

Good catch.

> Come to think of it, I'd probably also add the most obvious effect of :id
> for naming the server buffer, i.e. something like this:
>
>   When present, ID should be an opaque object for identifying the connection
>   unequivocally, and will correspond to name of the created erc server buffer
>   after connection. ...
>
> Can probably be phrased better, but since main effect of "(erc ...)" command 
> is
> creating that server buffer, what it will be named seem to be an important 
> detail
> (if only for finding it afterwards).
>
> [...]
>
> In fact, I think allowing for naming server buffers how you want them
> to be named with one easy and reasonably-obvious option should probably
> be most prominent thing about it:
>
>   https://e.var.nz/scr-20221215233955.jpg
>
> (example of using :id for all networks, in contrast to earlier
> inconsistent not-entirely-local naming)

Agreed. Thanks. I have attempted to change it to something along the
lines of what you've suggested. Hopefully it's passable.

> Pretty sure patches you've attached address main issue I wanted to raise
> (as well as couple other issues).
>
> Thanks again for doing all the work here.

My pleasure. I've added the changes to Emacs 29, so they should appear
on HEAD as well, shortly.

> And apologies for not being able to articulate main problem more clearly.
> Given how simple IRC is, should've definitely attached some easy-to-run
> protocol example to reproduce it in the first place (maybe as .eld test-case),
> instead of meandering description.

Nah. Your input has made all the difference. Bug reports, especially
good ones like this, are terribly undervalued as contributions, IMO.
Please consider contributing to ERC (in any form) again in the future.

Until next time!


--- End Message ---

reply via email to

[Prev in Thread] Current Thread [Next in Thread]