icecoder / ICEcoder

Browser code editor awesomeness
http://icecoder.net
Other
1.41k stars 348 forks source link

ICEcoder 'locks-up' stops opening files #980

Closed jenspr closed 3 years ago

jenspr commented 3 years ago

DESCRIPTION:

After a while (hours!!) of usage ICECODER (8.0 BETA) seems to lockup. It will not open files anymore when clicking files in the left navigation pane. (see errors below) The onyl workaround I found was to remove it and reinstall it. I guess something locked up :-( Any hint?

WEBSERVER: IIS/Windows BROWSERS: CHrome, Safari

CONSOLE OUTPUT:

icecoder.js?microtime=1624017804.6947:3498 GET https://scbuwebpoc-nynas.msappproxy.net/icecoder/lib/file-control.php? action=load&file=C&csrf=6cd63545accfb2eea6867ef936146cc4&lineNumber=/inetpub|wwwroot|slog|index.php 500 (Internal Server Error)

FROM ICECODER ERROR LOG:

[18-Jun-2021 12:05:13 UTC] PHP Fatal error: Uncaught TypeError: substr() expects parameter 3 to be int, bool given in C:\inetpub\wwwroot\ICEcoder\classes\File.php:47 Stack trace:

0 C:\inetpub\wwwroot\ICEcoder\classes\File.php(47): substr('C', 0, false)

1 C:\inetpub\wwwroot\ICEcoder\lib\file-control.php(64): ICEcoder\File->check()

2 {main}

thrown in C:\inetpub\wwwroot\ICEcoder\classes\File.php on line 47

[18-Jun-2021 12:05:24 UTC] PHP Fatal error: Uncaught TypeError: substr() expects parameter 3 to be int, bool given in C:\inetpub\wwwroot\ICEcoder\classes\File.php:47 Stack trace:

0 C:\inetpub\wwwroot\ICEcoder\classes\File.php(47): substr('C', 0, false)

1 C:\inetpub\wwwroot\ICEcoder\lib\file-control.php(64): ICEcoder\File->check()

2 {main}

thrown in C:\inetpub\wwwroot\ICEcoder\classes\File.php on line 47

mattpass commented 3 years ago

Thanks for the issue report & details, very helpful!

It would seem that when getting the file location for the file you're trying to load (in this case C), it can't seem to figure things out, causing substr('C', 0, false) here and the 3rd param must be an integer, not boolean: https://github.com/icecoder/ICEcoder/blob/8.0beta/classes/File.php#L47

Because the 3rd param on that line: strrpos(str_replace($docRoot, "", $file), "/") is returning false (because it can't find the string reverse position and ICEcoder's code expects a number), it's then causing a 500 error. It's weird you end up needing to reinstall icecoder but I suspect there's a knock-on reason. I've got something slightly wrong here and needs a tweak to always return a number.

Probably because it's an unusually short filename, C - I'll do some tests and see what needs amending.

jenspr commented 3 years ago

I can add following feedback. I had experienced the issue on IIS. I workaround the problem by disabling "auto open files" in the settings. My second install was on a Linux powered Webserver (don't know which). There everything worked as expected (=no issue) Hope this helps a bit.

StillTravelling commented 3 years ago

I have the same issue on windows. I don't believe the above example is where the file name is 'C'. That is the hard disk and the rest of the path is missing, i think probably due to the :. Except in my usage i edit files on the F drive and I get this issue too, except where C is the above, F shows.

ICEcoder.previousFiles = ['F:|xampp|htdocs|st|index.php']

The issue can be replicated by a new installation, Opening a file. Refreshing the browser twice, so ICEcoder.previousFiles refreshes with the data, then attempting to open from the recent files.

I think the issue is also impacting indexer.php as this always returns empty Uncaught (in promise) SyntaxError: Unexpected end of JSON input at icecoder.js?microtim…1624619945.7505:163

return response.json();

mattpass commented 3 years ago

Thank you both the updates and yeah @StillTravelling - you're right, the cause of the problems is the colon, not a short filename.

The cause was actually this... when you open a file you can optionally specify the line number to jump to if you wish, seperated by colon. ie, you can press CTRL + O and then if you have an /xyz.txt file you can enter /xyz.txt:5 in the popup dialog box and it'll load xyz.txt and set your cursor on line 5 when loaded.

However, the current logic it has re that colon separator doesn't take into account Windows paths which themselves have a colon, so when trying to open a Windows path it'll split C:/some/dir/xyz.txt into 2 parts - thinking C is the file path and /some/dir/xyz.txt is the line number.

I've pushed this commit to add extra logic, to fix that issue: https://github.com/icecoder/ICEcoder/commit/487ce2710cdbd23d246cdbcdefa21eac8d55aa89 (see lines 1993 to 2001).

This appears to fix the problem (I've seen it fixed now when running on XAMPP on Windows) - could you confirm all if OK for you both too?

@StillTravelling - I think the extra issue you noted re indexer is a knock on error. Now the above is fixed, it seems to run fine, let me know if you still see problems there?

StillTravelling commented 3 years ago

@mattpass Looks good now on windows. Can open recent files again once the page is refreshed, so many thanks.

The indexer still returned an empty response, but had a feeling it is actually related to something else. In the /data/index.php file, i can see it's been indexing .ts (media file rather than typescript) files which i think might be stressing the process out a bit. I have now removed ts from $indexableFileExts and it's now returning json data. Not sure this is fixable or worthy of fixing, just one of those things where the file extensions are the same.

mattpass commented 3 years ago

@StillTravelling glad to know the fixed worked.

Ah, didn't even consider file extensions may be used for different purposes, such as .ts. I think a good solution is to check if the file MIME type is "text/*" to avoid looking into binary files such as the media file you mentioned. Have done that in this commit: https://github.com/icecoder/ICEcoder/compare/42d60ea48fdb1a993232ea2685c99548ee1a8007...master

Hopefully you can now undo removing it from $indexableFileExts, pull test latest commits and it'll avoid scanning your .ts media files as they're not text from here on.

StillTravelling commented 3 years ago

@mattpass the indexer works perfectly now thanks to the MIME type addition. Thanks once again.

mattpass commented 3 years ago

Excellent, glad all is sorted here, will close issue now. Thanks for the help & feedback. 👍🏼