quoid / userscripts

An open-source userscript manager for Safari
https://quoid.github.io/userscripts/
GNU General Public License v3.0
3.36k stars 192 forks source link

Improve Error Logging #279

Open quoid opened 2 years ago

quoid commented 2 years ago

Broadly, error logging needs to be improved. Specifically, errors that are delivered in the popup are not very useful.

quoid commented 2 years ago

related #276

quoid commented 2 years ago

On the popup, it's hard to get more information about the general error shown. Perhaps the popup error can show an Error View with more info and even get the last n lines of the console output printed to that view.

Emasoft commented 2 years ago

There are a lot of websites where the userscripts just stop working, and those errors seem to break the extension too.
On iOS all you get is this cryptic error message:

When this message ("Something went wrong") appears, clicking to retry (even many times) is usually useless.
You don't know WHICH script crashed, nor WHY it crashed.

3 things would be very useful to add:

quoid commented 2 years ago

@Emasoft

Please provide device details to further help you with your issue: hardware, ios version etc...

That error you are seeing is most likely unrelated userscript execution errors. The error you see, likely happens when one of the popup init function has an unresolved promise, which often means something went amiss on the swift/os side. The error that is caught is logged to the console.

cryptic error message

There's not much room in the popup to add full details of the error log and probably not useful to most users. That's why the error is usually fully logged in the console.

In my experience hitting the retry link often resolves it, but obviously not always the case.

As far as error logging improvements go, which is likely unrelated to your issue:

The error message should report the exact javascript compiler error, including row and column.

Most errors are printed to the console as mentioned above. In order to access the console you'll need to plug your phone into your computer. The console will have the details of the error, for most errors, however in this case those details will probably not be useful.

The javascript instance should be isolated and not breaking the entire extension

Userscripts are isolated from each other and the failure of one script doesn't necessarily mean that all userscripts will fail to execute. That depends on how the user decides to inject their userscripts.

Again, this error you are seeing is different than a simple userscript failing to execute.

If a script crashes after the injection, the script that caused the error must be deactivated automatically, with a red icon (like this: 🚫) added as prefix of the script name in the list. Only when it compiles with no errors the script should be reactivated.

This is a big ask and feature to implement. I don't have plans for anything like this, but if you'd be willing to contribute, let me know.

Crashes are often linked only to specific websites. But userscripts deactivation is currently global. It should be deactivated only for the current website or domain. The extension should remember the user settings for a website/domain and activate or deactivate the scripts according to the user input.

https://github.com/quoid/userscripts/issues/225

Emasoft commented 2 years ago

@quoid

Please provide device details to further help you with your issue: hardware, ios version etc...

That error you are seeing is most likely unrelated userscript execution errors. The error you see, likely happens when one of the popup init function has an unresolved promise, which often means something went amiss on the swift/os side. The error that is caught is logged to the console.

I'm using an iPad Pro (macOS-15.5-iPad8,11-arm-64bit-1TB). I have a 2Tb iCloud Drive, and the Userscripts folder is in the ⁨path: "iCloud Drive⁩ ▸ ⁨Userscripts⁩". It contains 49 js userscripts.

Almost 50% of the websites I navigate give me the crash issue.

Unfortunately I cannot access the console log, because my computer is a linux machine and even libimobiledevice stopped working a long time ago after one of the latest iOS updates. I can only use the iPad. Is there a way to get a more meaningful error message?

quoid commented 2 years ago

Is there a way to get a more meaningful error message?

Not without having access to a computer running macOS. However, if you look at this issue you will see the error message that, usually, gets logged to the browser console and os console when that error appears.

As you can see, the error that gets log is itself quite vague:

Error for init promise: Error: Error calling runtime.sendNativeMessage(): Error Domain=SFErrorDomain Code=3 "(null)". (popup.js, line 4167)

As far as I know that error can mean a number of things, but the only time I've seen it myself was when the memory limit was being hit before Apple increased it. in 15.0 it was 6mb but increased to 80mb in 15.1.

I don't know if we can be certain it's the memory limit being hit, but it is certainly possible. Reducing the overall memory footprint of the extension would be useful regardless.

Emasoft commented 2 years ago

Memory footprint? I have ~8 Mb of .js files in the userscripts folder. But some scripts load icons or external libs from remote servers. Is this enough to get over the 80Mb limit?

quoid commented 2 years ago

@Emasoft it's possible, especially if all those scripts are being loaded at the same time, but this a guess since we can't properly debug the device

also, when I talked about reducing the memory footprint, i meant to further optimize the ios extension where/if possible

Emasoft commented 2 years ago

@quoid Is some form of memory control possible? What about loading the .js files one by one and counting the bytes added until they reach a predetermined total limit, and then stop loading the remaining ones? You can make the limit configurable by the user, so each one can calibrate on his device.

quoid commented 2 years ago

@Emasoft I guess that an option, but I'd like to first determine what is indeed causing these types of issues and if it possible to repro hitting a memory limit on my own devices (iPhone 8) and take it from there

Let's take any further chat about this into it's own issue: #293

ACTCD commented 2 years ago

We may should also bring some errors of background page to the front UI.

For example, I write a REQ script on the extension page, when I save the script, the following errors appear in the console of the background page, which may be difficult for ordinary users to perceive the existence of those errors, probably just stuck in a quandary that why the script doesn't work.

Since I'm using the key regexSubstitution which may not be supported by Safari, it outputs an error like this:

Error setting session rules: Error: Invalid call to declarativeNetRequest.updateSessionRules(). Error with rule at index 0: Rule with id 460 is invalid. `redirect` is missing either a `url`, `extensionPath`, or `transform` key.

I was using comments in the script and I didn't realize it had to be in JSON format, causing errors like:

Unhandled Promise Rejection: SyntaxError: JSON Parse error: Unrecognized token '/'
Unhandled Promise Rejection: SyntaxError: JSON Parse error: Single quotes (') are not allowed in JSON
dimitarvp commented 1 year ago

I just got "failed to get remote content, invalid url" when trying to import https://github.com/quenhus/uBlock-Origin-dev-filter/blob/main/dist/userscript/google_duckduckgo/all.txt or https://raw.githubusercontent.com/quenhus/uBlock-Origin-dev-filter/main/dist/userscript/google_duckduckgo/all.txt on the latest macOS (13.4.1) and Safari (16.5.1).

If that's not the real error I would really love to find out what it is. Here's a screenshot of Safari's console:

Screenshot 2023-07-03 at 16 47 09

Hopefully this isn't a basic user error when trying to import but I wouldn't know because I only see one line of explanation on how to add a remote filter list and it's pretty straightforward. Maybe the problem is that the file extension is not .js?

quoid commented 1 year ago

@dimitarvp

Maybe the problem is that the file extension is not .js?

Yep - https://github.com/quoid/userscripts/blob/4a4c257f14c98cf55115f6cb9c258946efd6d21c/xcode/Safari-Extension/Functions.swift#L167

dimitarvp commented 1 year ago

Yikes. Is there a chance to modify the "Add remote" UI to allow all file extensions and also allow specifying what kind of remote file you are adding (CSS / JS)?

quoid commented 1 year ago

Yikes. Is there a chance to modify the "Add remote" UI to allow all file extensions and also allow specifying what kind of remote file you are adding (CSS / JS)?

I'd lean more towards allowing txt, when adding from remote, rather than all file extensions. I think that would require updating the validateUrl func linked above. Since that func is used elsewhere, maybe an optional arg to also allow txt when the PAGE_NEW_REMOTE message is received?

Alternatively, you could just copy and paste the code into a new entry (css/js). The Add Remote option is really just a convenience, it doesn't offer any special features over manual creation.

dimitarvp commented 1 year ago

I'd lean more towards allowing txt, when adding from remote, rather than all file extensions.

That works too. Whatever is lowest friction and minimum risk for you.

The Add Remote option is really just a convenience, it doesn't offer any special features over manual creation.

Does it not automatically and periodically update the script when it's added as an URL? If not then yeah, just adding it inline is a fire-and-forget operation. But if it actually updates the script I'd still prefer that option -- if adding the above functionality is viable.

ACTCD commented 1 year ago

In my opinion, there should be no support for txt or other suffixes, that would create more confusion. In fact we should probably restrict it more strictly to .user.js and .user.css.

If the source is provided as txt or any other format, it is not intended to be served as a user script, which means potential failure or invalidation or error.

Does it not automatically and periodically update the script when it's added as an URL?

Yes, currently we don't record install URL in any form, so there will be no updates. But this is planned to be supported in the future and tracked in #248.

That is, update checking is currently implemented via @updateURL and @downloadURL in the metadata.

dimitarvp commented 1 year ago

I see. Well I already set it up inside my iCloud Drive in a dedicated directory and can see the userscript I wanted (and linked above) working both on my Mac and iPhone so I am good.

IMO you should eventually imitate uBlock Origin however: namely allow the users to add URLs and periodically pull them and update the DB with their changes.

For now I am good but I hope these last few comments outline a potential misunderstanding in the docs (i.e. when I see "Add URL" to me this automatically means "this URL will periodically be pulled and the local DB will be updated with its contents") and maybe a future feature. Your call.

Thanks for clearing up the confusion. ❤️