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

[kepstin]

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 331d9f7. 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.

[jaraco]

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]

As I think about it more, I don't see a good way to write tests to capture expectations about whether events are numeric or named. I could imagine putting some comments in the events module to signal not to change those without making a breaking release, but even then, it seems easy enough to miss, especially in a diff. I could also imagine a test that grabs the latest release and compares the events between the latest release and the current head, and have it raise a warning if there's a difference (and signal to cut a breaking release). That doesn't sound easy or cheap... and also could be easily missed, as warnings don't stop a merge. Having the condition be an actual failure might do the trick, but then we'd have to figure out how not to fail when a breaking change is acknowledged/planned. With the current towncrier model, the test suite might be able to check for newsfragments/*.removal.*, but that's pretty brittle and entangles the test suite with the release process and changelog management.

What would be better is if:

• Events would be strings and codes. (e.g. Event('welcome', 1))
• Comparing an event to a string should work: event == 'welcome'
• Comparing an event to a number should work: event == 1
• (maybe) Comparing an event to a numeric string should work: event == '001'
• Things like expect should be implementable with any supported form (e.g. self.conn.expect([903, "904", "saslmechs"])). (maybe not "904).
• Unknown codes (those not yet implemented) should be represented as integers (assuming the spec guarantees that they are integers).

[jaraco]

In the 20.5.0 release, I've refactored the command handling so that commands now retain both the code and the name (or use the code as the name when only the numeric code is present). This approach should mean that clients like heisenbridge should be able to rely on the numerics for the logic where names are undefined. If this functionality had been present earlier, heisenbridge could have avoided the risk of breakage by using command.code in the expect call.

While this can't fix the compatibility issue for heisenbridge going back in time, it does provide a mechanism for libraries to future-proof themselves.