fix(line): route inbound media to matching caches - #34233
Conversation
|
Thanks for working on this. I hit this exact bug from a live LINE image webhook on current Sanitized traceback from the gateway log: I also checked this PR locally in a detached worktree: A merge-tree probe against current |
|
Thanks for the pointer. I opened this from a live LINE webhook crash on current main and kept it focused on the MessageType.IMAGE failure plus the related cache-helper mismatch for non-image media. I agree this overlaps with #27142. The main differences here are:
If maintainers prefer #27142 as the canonical PR, I’m happy to close this one or help move the tests/evidence there. Otherwise this PR is ready from my side. |
d462bcf to
4fdf7c1
Compare
|
Updated this PR after upstream commit The PR is now narrowed to the remaining inbound media cache issue only:
So this is no longer intended to duplicate the Local verification after the rebase/scope reduction: |
Correcting an earlier Re-classifying as related_to #27142 and #44019 (same LINE cache-routing cluster), not a duplicate. Maintainer pick: #34233 is mergeable against current main with focused tests; #27142's MessageType half is already merged; #44019 also adds outbound media serving. |
GottZ
left a comment
There was a problem hiding this comment.
This was generated by AI during triage.
Summary
Two open PRs address the remaining LINE inbound-media defect after the MessageType mapping fix landed on main: both replace the incorrect image-only cache path with typed cache helpers. #34233 additionally preserves Content-Type for MIME-aware extensions and downstream media types, while #35785 preserves LINE file names but includes unrelated author-map churn.
Related pull requests
- #34233
related— (+116/-12) — preferred consolidation target: Routes image, audio, video, and file payloads to the established typed cache helpers, preserves response Content-Type for extension selection, emits MIME-style media types, and adds focused production-path regressions. - #35785
duplicate— (+125/-10) — duplicate core fix with a useful filename detail: After responding to the contributor keep_open review by removing the redundant generic cache API and adding production-path tests, the diff now implements the same typed-helper routing as #34233; its distinct useful behavior is forwarding LINE fileName, while scripts/release.py attribution changes are unrelated to the reported defect.
Duplicates
#34233 and #35785 now substantially duplicate the same LINE inbound image/audio/video/file cache-routing fix and regression coverage.
Suggested consolidation
Merge #34233 as the narrower, MIME-aware consolidation target, preferably carrying over #35785's fileName forwarding and corresponding regression before merge; then close #35785 as superseded. Despite the keep_open review on #35785, the requested salvage has been completed, and the resulting production diff now overlaps #34233 rather than providing an independent fix.
Complex graph
flowchart LR
classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
classDef best stroke-width:3px,stroke:#b45309
classDef target stroke-width:3px,stroke:#4338ca
subgraph Dup34233 ["PRs duplicating each other"]
P34233["PR #34233 (open)"]
P35785["PR #35785 (open)"]
end
class P34233 open
class P35785 open
class P34233 target
click P34233 "https://github.com/NousResearch/hermes-agent/pull/34233"
click P35785 "https://github.com/NousResearch/hermes-agent/pull/35785"
Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed or no verify verdict yet (state tag in the node label).
Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 18 kB of PR diffs, 2 kB of issue/PR text, 9 kB of discussion (12 comments), 1 verify verdict. verdicts reflect diff content, not PR titles. Part of an automated triage batch.
|
Closing this PR because its main fixes have now been superseded upstream.
The remaining difference here is Content-Type-based extension inference. I do not currently have a production case that justifies maintaining a separate patch for that narrower improvement, so I am intentionally not carrying that delta forward. If a concrete MIME/extension failure appears, I can submit a focused current-main PR. Thanks to everyone who reviewed and helped triage this. |
Summary
This PR addresses the inbound media crashes and issues on the LINE Messaging API platform adapter:
image,audio,video,file,sticker,location) to standard gatewayMessageTypeenums instead of forcingMessageType.IMAGE(which does not exist and raisedAttributeError: IMAGE).cache_audio_from_bytes,cache_video_from_bytes,cache_document_from_bytes) based onmsg_typeinstead of writing everything tocache_image_from_bytes(which rejected non-image payloads)._LineClient.fetch_content()to return theContent-Typeheader, enabling_download_mediato guess correct extensions viamimetypes.guess_extensioninstead of forcing.bin.tests/gateway/test_line_plugin.pyto cover media download/caching and mapping behavior.