Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Search - TypeError: 'NoneType' object has no attribute '__getitem__' #640

Closed
FraserChapman opened this issue Jul 13, 2019 · 8 comments
Closed

Comments

@FraserChapman
Copy link

Error when trying to search

file: plugin.video.youtube/resources/lib/youtube_plugin/kodion/utils/search_history.py
"list" method of the "SearchHistory" class, line 32:

result.append(item[0]) throws TypeError: 'NoneType' object has no attribute 'getitem'

It appears that the "_get" method of the "Storage" class at: plugin.video.youtube/resources/lib/youtube_plugin/kodion/utils/storage.py returns "None" in some cases - but the line causing the error never checks to see if "item" is None - giving the error when item is indexed.

There is further discussion and a log file on this thread: https://forum.kodi.tv/showthread.php?pid=2868050#pid2868050

@FraserChapman
Copy link
Author

Ok I think I have found the root of the error - it appears that the BLOB convertor I register in script.module.cache is being used by the YouTube add-on!?

If I add

sqlite3.register_converter("BLOB", None)

to the file: plugin.video.youtube/resources/lib/youtube_plugin/kodion/utils/storage.py

I no longer get any errors...however why the sqlite3 instance in YouTube is using the Blob convertor in script.module.cache is very puzzling - I will look into removing this or else ensuring the convertor is unregistered when script.module.cache is disposed.

Bizarre...

@anxdpanic
Copy link
Collaborator

Yea, registering converters seem to apply globally, when detect_types is on. This has happened with another add-on in the past but with the TIMESTAMP column.
Changing https://github.com/jdf76/plugin.video.youtube/blob/master/resources/lib/youtube_plugin/kodion/utils/storage.py#L54 to detect_types=0 resolves the issue with some extra handling for timestamp conversion.

@anxdpanic
Copy link
Collaborator

6d6cb7f works around the issue in this add-on. Looks like register_converter should be avoided when possible.

@FraserChapman
Copy link
Author

FraserChapman commented Jul 14, 2019

@anxdpanic - just thinking about this more - you could "harden" the YouTube plugin against such occurrences/usages of register_converter by simply defining your own passthrough converters in the relevant modules. This would ensure that any such future use - by any add-on - would have zero effect (as the BLOB converter is currently doing within script.module.cache)

i.e. in plugin.video.youtube/resources/lib/youtube_plugin/kodion/utils/storage.py

    def _open(self):
        if self._file is None:
            self._optimize_file_size()

            path = os.path.dirname(self._filename)
            if not os.path.exists(path):
                os.makedirs(path)
            # "passthrough" converters to harden against 
            # register_converter calls in other add-ons
            sqlite3.register_converter("TIMESTAMP", lambda _: _)
            sqlite3.register_converter("BLOB", lambda _: _)
            self._file = sqlite3.connect(self._filename, check_same_thread=False,
                                         detect_types=sqlite3.PARSE_DECLTYPES, timeout=1)

            self._file.isolation_level = None
            self._cursor = self._file.cursor()
            self._cursor.execute('PRAGMA journal_mode=MEMORY')
            self._cursor.execute('PRAGMA busy_timeout=20000')
            # self._cursor.execute('PRAGMA synchronous=OFF')
            self._create_table()

Happy to patch and create a PR if this is something that you think is worth it

(FWIW I have altered my usage to stop the issue - my thinking here is to stop it ever happening again)

@anxdpanic
Copy link
Collaborator

This shouldn't be an issue in the future. 6d6cb7f sets detect_types to 0, so any registered converters shouldn't be used now.

@FraserChapman
Copy link
Author

Ah of course, sorry, kind of misconstrued what you meant. Good call :)

@anxdpanic
Copy link
Collaborator

Thanks for your help with the issue :)

@FraserChapman
Copy link
Author

np :)

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

No branches or pull requests

2 participants