fix: move unreachable event cases out of the message content switch - #32
Merged
Conversation
MemberJoinedEvent, MemberLeftEvent, FollowEvent and BeaconEvent were cases on the inner `switch e.Message.(type)`. They are event types, not message contents, so they could never match at runtime. This compiled only because MessageContentInterface is `GetType() string`, which the event types happen to satisfy, so the compiler could not flag them as impossible type switch cases. Moving them to the outer switch over cb.Events exposes a second bug: every case asserted `e.Source.(webhook.UserSource)`, but Source is a SourceInterface whose concrete type depends on the chat. Member joined/left only occur in group and multi-person chats, so the source is a GroupSource or RoomSource and the assertion panics. - Move the four cases to the outer event switch - Add sourceID to describe user, group and room sources without panicking - Report the joined/left members from e.Joined / e.Left rather than the source, and the hwid from e.Beacon - Guard the optional pointer sub-objects with orZero so a payload that omits them logs instead of crashing - Fix the copy-pasted "Member joined" log on the member left case - Add tests covering the decoded payload shape and the helpers
kkdai
force-pushed
the
fix/unreachable-event-cases
branch
from
September 1, 2026 07:25
af6c195 to
ffaeed7
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
main.gohandled four event types as cases on the inner switch:These are event types, not message contents, so they could never match. Normally the compiler rejects an impossible type switch case, but it can't here:
MessageContentInterfaceis justand the event types happen to have a
GetType() stringtoo, so they satisfy it structurally. Four silently dead branches in a template people copy from.A second bug underneath
Moving them to the outer switch is not enough — every case asserted
e.Source.(webhook.UserSource), butSourceis aSourceInterfacewhose concrete type depends on the chat. Member joined/left events only occur in group and multi-person chats, so the source is aGroupSourceorRoomSource. The unchecked assertion panics, and a panic in anhttp.HandlerFunckills the request:(reproduced against a real
memberJoinedpayload — seeTestOldCodePanicsreasoning captured inTestMemberJoinedIsEventNotMessageContent).The fix
FollowEvent,MemberJoinedEvent,MemberLeftEventandBeaconEventto the outer switch overcb.EventssourceIDto render user / group / room sources without asserting a single concrete typee.Joined/e.Left, the beacon hwid frome.Beacon— the source alone was never the interesting fieldorZeroso a payload that omits them logs instead of nil-derefing"Member joined"log message on the member left case"Unsupported message: %T"to"Unsupported event: %T", which is what it actually reportsTests
First tests in this repo.
main_test.godecodes a realmemberJoinedwebhook payload and pins down both facts the old code got wrong — that it is an event rather than a message content, and that its source is aGroupSource— plus table tests forsourceID,userIDsand the nil-pointer paths.go build ./...go vet ./...gofmt -l .cleango test ./...— 4 tests, all passingNote
Rebased on top of
masterafter #31 (SDK v8.22.0 + README) was merged, so CI here runs against Go 1.25 and line-bot-sdk-go v8.22.0.