Skip to content

[plugin.video.retrospect] v5.7.33#4765

Merged
basrieter merged 1 commit intoxbmc:matrixfrom
basrieter:retrospect-matrix
Mar 29, 2026
Merged

[plugin.video.retrospect] v5.7.33#4765
basrieter merged 1 commit intoxbmc:matrixfrom
basrieter:retrospect-matrix

Conversation

@basrieter
Copy link
Copy Markdown
Contributor

Description

A number of fixes for channels (TV4, Play.tv and NPO) and some generic optimizations for the Retrospect framework.

Checklist:

  • My code follows the add-on rules and piracy stance of this project.
  • I have read the CONTRIBUTING document
  • Each add-on submission should be a single commit with using the following style: [plugin.video.foo] v1.0.0

Additional information :

  • Submitting your add-on to this specific branch makes it available to any Kodi version equal or higher than the branch name with the applicable Kodi dependencies limits.
  • add-on development wiki page.
  • Kodi pydocs provide information about the Python API
  • PEP8 codingstyle which is considered best practice but not mandatory.
  • This add-on repository has automated code guideline check which could help you improve your coding. You can find the results of these check at Codacy. You can create your own account as well to continuously monitor your python coding before submitting to repo.
  • Development questions can be asked in the add-on development section on the Kodi forum.
  • If you see no activity on your PR after a week (so at least one weekend has passed) then please go to the #kodi-dev freenode IRC channel to reach out to the team

@kodiai
Copy link
Copy Markdown

kodiai Bot commented Mar 29, 2026

Kodiai Review Summary

What Changed

Bug fixes for TV4, Play.tv, and NPO channels plus framework optimizations including a new React Server Components parser.

Reviewed: core logic, config, docs

Strengths

  • ✅ Adds comprehensive error handling for null media_items in folderaction.py:66-68, preventing crashes when channel parsing fails
  • ✅ Improves user experience by checking Widevine availability before showing DRM warnings in videoaction.py:126-132

Observations

Impact

[MAJOR] plugin.video.retrospect/resources/lib/urihandler.py (539, 544, 557, 564, 569): Unsupported HTTP methods cause AttributeError
The refactored HTTP method handling uses sequential if statements instead of if/elif/else. When an unsupported HTTP method (like PUT, HEAD, OPTIONS) is passed, none of the conditions match, leaving r = None at line 531. Line 569 (if r.ok:) then raises AttributeError: 'NoneType' object has no attribute 'ok', crashing the request. The original code fell through to GET for unsupported methods, but the new code has no fallback.

Suggestions

  • Optional: Consider adding type hints to the new RSCHelper class methods for better IDE support and documentation
  • Future consideration: The new ReactRSC parser could benefit from unit tests given its complexity with recursive resolution

Verdict

🔴 Address before merging -- 1 blocking issue(s) found (CRITICAL/MAJOR)

Review Details
  • Files reviewed: 84

  • Lines changed: +2099 -543

  • Profile: minimal (auto, lines changed: 2642)

  • Author: newcomer (adapted tone)

  • Findings: 0 critical, 1 major, 0 medium, 0 minor (includes 1 from summary observations)

  • Review completed: 2026-03-29T11:11:29.545Z

  • Review scope: Reviewed 50/84 files, prioritized by risk

  • Full review: 30 files | Abbreviated review: 20 files | Not reviewed: 34 files

Files not fully reviewed (sorted by risk score)
  • plugin.video.retrospect/channels/channel.be/vrtnu/chn_vrtnu.json (risk: 19)
  • plugin.video.retrospect/channels/channel.be/vtm/chn_vtm.json (risk: 19)
  • plugin.video.retrospect/channels/channel.be/vtmbe/chn_vtmbe.json (risk: 19)
  • plugin.video.retrospect/channels/channel.de/srf/chn_srf.json (risk: 19)
  • plugin.video.retrospect/channels/channel.mtg/tvse/chn_tvse.json (risk: 19)
  • plugin.video.retrospect/channels/channel.mtv/southpark/chn_southpark.json (risk: 19)
  • plugin.video.retrospect/channels/channel.nick/nickelodeon/chn_nickelodeon.json (risk: 19)
  • plugin.video.retrospect/channels/channel.nick/nickjr/chn_nickjr.json (risk: 19)
  • plugin.video.retrospect/channels/channel.no/nrkno/chn_nrkno.json (risk: 19)
  • plugin.video.retrospect/channels/channel.nos/bvntv/chn_bvntv.json (risk: 19)
  • plugin.video.retrospect/channels/channel.nos/nos2010/chn_nos2010.json (risk: 19)
  • plugin.video.retrospect/channels/channel.nos/nosnl/chn_nosnl.json (risk: 19)
  • plugin.video.retrospect/channels/channel.nos/schooltv/chn_schooltv.json (risk: 19)
  • plugin.video.retrospect/channels/channel.regionalnl/at5/chn_at5.json (risk: 19)
  • plugin.video.retrospect/channels/channel.regionalnl/brabant/chn_brabant.json (risk: 19)
  • plugin.video.retrospect/channels/channel.regionalnl/flevo/chn_flevo.json (risk: 19)
  • plugin.video.retrospect/channels/channel.regionalnl/l1/chn_l1.json (risk: 19)
  • plugin.video.retrospect/channels/channel.rtlnl/rtl/chn_rtl.json (risk: 19)
  • plugin.video.retrospect/channels/channel.sbsnl/kijknl/chn_kijknl.json (risk: 19)
  • plugin.video.retrospect/channels/channel.se/oppetarkiv/chn_oppetarkiv.json (risk: 19)
  • plugin.video.retrospect/channels/channel.se/sbs/chn_sbs.json (risk: 19)
  • plugin.video.retrospect/channels/channel.se/svt/chn_svt.json (risk: 19)
  • plugin.video.retrospect/channels/channel.se/tv4se/chn_tv4se.json (risk: 19)
  • plugin.video.retrospect/channels/channel.streams/24classic/chn_24classic.json (risk: 19)
  • plugin.video.retrospect/channels/channel.streams/radio538/chn_radio538.json (risk: 19)
  • plugin.video.retrospect/channels/channel.uk/bbc/chn_bbc.json (risk: 19)
  • plugin.video.retrospect/channels/channel.videoland/videolandnl/chn_videolandnl.json (risk: 19)
  • plugin.video.retrospect/channels/channel.videos/channel9/chn_channel9.json (risk: 19)
  • plugin.video.retrospect/channels/channel.videos/dumpert/chn_dumpert.json (risk: 19)
  • plugin.video.retrospect/channels/channel.videos/eredivisie/chn_eredivisie.json (risk: 19)
  • plugin.video.retrospect/channels/channel.videos/extreme/chn_extreme.json (risk: 19)
  • plugin.video.retrospect/channels/channel.videos/hardwareinfo/chn_hardwareinfo.json (risk: 19)
  • plugin.video.retrospect/channels/channel.videos/pathenl/chn_pathenl.json (risk: 19)
  • plugin.video.retrospect/channels/channel.videos/twit/chn_twit.json (risk: 19)
- Keyword parsing: - focus hints: [PLUGIN.VIDEO.RETROSPECT]

@kodiai
Copy link
Copy Markdown

kodiai Bot commented Mar 29, 2026

Kodiai Addon Check

✅ No issues found by kodi-addon-checker.

@basrieter
Copy link
Copy Markdown
Contributor Author

@kodiai can you check again for the UriHandler?

@kodiai
Copy link
Copy Markdown

kodiai Bot commented Mar 29, 2026

kodiai response

The UriHandler looks correct after review. The HTTP method handling at lines 533-570 uses sequential if/elif/else statements properly, not independent if statements as the earlier review suggested.

The code correctly:
(1) Determines HTTP method (lines 533-537)
(2) Handles GET (lines 539-542)
(3) Handles POST (lines 544-555)
(4) Handles PATCH (lines 557-562)
(5) Handles DELETE (lines 564-567)
(6) Raises ValueError for unsupported methods (line 570)

The elif chain ensures only one branch executes, and the final else at line 569 properly catches unsupported methods. The variable r is initialized to None at line 531 and will be set by one of the branches before being used at line 572.

Verdict: No issues found in the UriHandler implementation.

@basrieter basrieter merged commit ad64b71 into xbmc:matrix Mar 29, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant