irc icon indicating copy to clipboard operation
irc copied to clipboard

The addition of new numeric event names for SASL broke heisenbridge, which was using numeric codes.

Open kepstin opened this issue 2 years ago • 1 comments

The release of irc 20.3.0 caused a regression in heisenbridge where SASL authentication no longer works (it hits a timeout in network_room.py#L1477-1502 when waiting for the authentication response).

The cause of this was the new event names added in https://github.com/jaraco/irc/commit/331d9f782ff606d49440b6eb485200322eb124a3. heisenbridge was waiting for the numeric events - await self.conn.expect(["903", "904", "908"]) - and those are no longer being emitted by the irc library, which is now emitting named events instead (saslsuccess, saslfail, saslmechs).

I'm not sure if there's anything that can be done in the irc library at this point, but I wanted to bring this issue to your attention. It might be helpful to treat adding new numeric event name mappings as a "breaking" change which results in a new major version of the library, or otherwise document that using numeric events directly is unstable, and that they might change in minor version updates.

kepstin avatar Aug 15 '23 17:08 kepstin

Apologies for the breakage. It was unintended, but you're right - had we realized there was a dependency on that behavior, we would have released it as a backward-incompatible change and possibly warned in advance. If you think it would be worthwhile, there are a few things we could still do:

  • Add tests capturing the current expectation of named events (so changing it again would flag a breaking change).
  • Add compatibility code so users expecting numeric codes could still be supported.
  • Revert the change in 20.4 and cut a new release of 21.0 with the breaking change.

It appears heisenbridge has already addressed the issue by handling the events, so I'm slightly inclined to do nothing. I'd very much welcome for someone to add tests or backward compatibility. Let me know what you think.

jaraco avatar Dec 25 '23 03:12 jaraco