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
the load
function does not fail the promise correctly
#255
Comments
shankari
added a commit
to e-mission/nrel-openpath-join-page
that referenced
this issue
May 11, 2023
- Create an invalid study div with some internal structure - error, error details, and contact for assistance - invalid div is hidden by default - Create a function that will hide the valid div and populate and show the invalid div - Call the function if there are issues while loading the dynamic config - use hardcoded values since the i18n might or might not be loaded correctly I originally assumed that the error would be generated even if the `i18n` load was not successful, but that is not the case wikimedia/jquery.i18n#255 I then tried to force such an error ``` // i18nLoadPromise always resolves with undefined // it explicitly catches failure responses and resolves instead // https://github.com/wikimedia/jquery.i18n/blob/6afc05ae64a2051d0a97b143ae90ca76f651874e/src/jquery.i18n.messagestore.js#L31 // I have filed an issue for this // wikimedia/jquery.i18n#255 // while waiting for it to be discussed, we need to look for a workaround // only current way to determine whether or not the load succeeded // is to check whether there are messages for the currently loaded language if (!$.i18n().messageStore.messages['en']) { throw SyntaxError("'en' i18n not found or unparseable"); } ``` but then realized that the join URL was only dependent on the study config and not the i18n, so even if the i18n was unsuccessful, we would be able to generate the join URL. So I removed it (although I have recorded it here for posterity). I also did not include a catch block in the second round of loading.
ANKITSINGH065
added a commit
to ANKITSINGH065/jquery.i18n
that referenced
this issue
Jun 8, 2023
please review @shankari |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
the
$.i18n().load
method returns a jquery promise.jquery.i18n/src/jquery.i18n.messagestore.js
Line 23 in 6afc05a
HOWEVER, if the load fails, the promise does not fail. The code explicitly traps the fail callback and converts it to a success callback. It appears that this is to handle 404 exceptions properly via fallbacks. But this also then does not catch other exceptions, such as invalid JSON.
It seems like it would be better to resolve only on the 404 exception, and reject on other exceptions.
I am happy to submit a PR if that would be welcomed.
The text was updated successfully, but these errors were encountered: