Edirom / Edirom-Online

Edirom Online is a tool for presenting historical-critical music editions in digital form.
GNU General Public License v3.0
20 stars 25 forks source link

Uncaught error if 'start_documents_uri' not defined in prefs #279

Closed bwbohl closed 3 weeks ago

bwbohl commented 2 years ago

app.js:1 Uncaught constructor {msg: 'No preference found with this key', key: 'start_documents_uri', level: 'warn', sourceMethod: 'getPreference', sourceClass: 'EdiromOnline.controller.PreferenceController', …}

bwbohl commented 2 months ago

This does not result in any malfunction but just doesn’t seem right and bad style.

bwbohl commented 1 month ago

Apparently it is sufficient if the prefs file contains an empty entry in the form:

<entry key="start_documents_uri" value=""/>
daniel-jettka commented 3 weeks ago

So this is where it may happen that the warning is thrown if there is no entry in the prefs file for start_documents_uri: (1) https://github.com/Edirom/Edirom-Online/blob/develop/app/Application.js#L130 (2) https://github.com/Edirom/Edirom-Online/blob/develop/app/Application.js#L183

What do you think @bwbohl - is there something to fix or a better way to handle things is the entry is not there?

bwbohl commented 3 weeks ago

I think if we add a check (probably in in (2)), wheter the return value of var uris is empty or null and if so return; that would do the job.

daniel-jettka commented 3 weeks ago

Where and when is the error message thrown anyway? :-)

Can there be an additional parameter here? - https://github.com/Edirom/Edirom-Online/blob/develop/app/Application.js#L185

So that var uris = me.getController('PreferenceController').getPreference('start_documents_uri');

becomes var uris = me.getController('PreferenceController').getPreference('start_documents_uri', true);

The definition of getPreference looks like that - https://github.com/Edirom/Edirom-Online/blob/develop/app/controller/PreferenceController.js#L64 getPreference: function(key, lax) { ... }

Would test that if I knew how to throw the error...

bwbohl commented 3 weeks ago

If you lad an edirom online and the edition has not defined a value for key start_documents_uri in its preferences the error is thrown. You located the right spoot in the JS that throws the error.

https://github.com/Edirom/Edirom-Online/blob/aadb46b7ce62c9523c09d6e393b5d5688654659d/app/controller/PreferenceController.js#L64-L94

to be exact:

https://github.com/Edirom/Edirom-Online/blob/aadb46b7ce62c9523c09d6e393b5d5688654659d/app/controller/PreferenceController.js#L83-L88

The error can be observed in the Developer Tools JS console of your browser.

bwbohl commented 3 weeks ago

This is not a critical issue, as you can see from the error level in line 87 which is set to warn. But I think it’s nicer to catch it before an warning is raised, e.g. by avoiding the execution if the preferences key is an empty string or null