nuprl / Ocelot

An IDE for JavaScript, without the "bad parts".
https://www.ocelot-ide.org/
GNU Lesser General Public License v3.0
30 stars 4 forks source link

IDE fails to save file / run code after idle time #146

Open mminea opened 3 years ago

mminea commented 3 years ago

This has been an issue for a while, reported by several users. After some inactivity in the browser window, the IDE no longer behaves properly. This typically involves

arjunguha commented 3 years ago

do you have a sense for how long I have to wait to witness this?

mminea commented 3 years ago

Sorry, I should have experimented and been more specific. I believe something like half an hour maybe. It has happened to me rather frequently, and several students have been reporting it, so it's not isolated.

dungwinux commented 3 years ago

For the second issue, I think that the program is still able to run correctly, but React failed to update and render the console properly.

https://github.com/ocelot-ide/Ocelot/blob/6887ff2/frontend/src/OutputPanel.tsx#L16-L42

Based on testing, I believe class ConsoleOutput is where the problem lies. React recommends each components to have unique keys, which help React identify which items have changed, are added, or are removed. But since there is no key in each ConsoleOutput instance, React cannot detect the changes.

Update: I found out that ConsoleOutput relies on console-feed to generate keys. By default, console-feed uses enumeration to generate identifiers, based on the object it was given at the time. This, however, has a side-effect when slicing the logs. If a console has 100 consecutive lines rendered with key from log-0 to log-99, then by adding another log line and slicing it, console-feed still sees it as 100 consecutive lines and renders with key from log-0 to log-99. The problem can be fixed by creating custom id tracker and adding custom id property to each Message before giving to console-feed

arjunguha commented 3 years ago

This is about the auto-save issue. I'll look into the console-not-refreshing problem in a bit.

I have not been able to reproduce the issue described. Here is what I did. I had Ocelot open in the background for several hours. I make edits to the buffer at 10:40AM, 11:50AM, and then again at 13:30PM. The file was successfully auto-saved at all times.

Let me know if you think I should do a different experiment. If not, would you just email me immediately when you encounter this problem again? Ideally, you would include the web browser console log. I'll look at the server logs to see what's up.

arjunguha commented 3 years ago

@dungwinux I'm pretty sure you're right. Thanks for the suggestion. I can prepare the fix. Or, would you rather submit a patch?

arjunguha commented 3 years ago

It should be possible to host Ocelot on localhost to test a change:

https://github.com/ocelot-ide/Ocelot/blob/master/frontend/package.json#L50

dungwinux commented 3 years ago

@arjunguha I can submit a PR.

I have one question: is it alright if I add an additional, zero-dependency package for the sake of id randomness?

arjunguha commented 3 years ago

You can. But, how about cycling through a set of IDs. I think scrollback is set to 100 lines. Would it be sufficient to cycle through 200 IDs?

arjunguha commented 3 years ago

I have to warn you: when it comes to code, if you ask me for an opinion, I will always have one. :)

But, just submit what you think is best and we can talk about it. One new dep is fine. Ocelot already depends on millions of packages. :(

arjunguha commented 3 years ago

@mminea I notice that Ocelot has a three hour maximum session length. Do you think you're witnessing this problem after three hours of inactivity? If that's the case, I can extend the session length.

On the other hand, if saving fails sooner than three hours, it's still a problem.

mminea commented 3 years ago

Oh, that would explain it. I have definitely revisited a window after more than three hours. I could imagine that students also keep a window open and work on and off. Perhaps extending the session length would be good. I'll watch out if saving fails after a shorter time, thanks!

arjunguha commented 3 years ago

I just updated the session length to 12 hours. I don't want to eliminate the session length cap. But, I think what's needed is a better message (e.g., you've been logged off, but don't worry, your work is all saved.)

arjunguha commented 3 years ago

So, I guess I broke my rule and made an update. But, it should really be "only" an update to the session length, so nothing should go wrong. Let me know if you notice anything.