thomasloupe / Slackord

Slackord is an application that allows you to migrate Slack messages into a Discord server.
https://thomasloupe.com
GNU General Public License v3.0
159 stars 22 forks source link

If Slackord crashes, restarting an import will create a new Disord category and channels instead of using previous ones it has already made #95

Closed ScriptAutomate closed 7 months ago

ScriptAutomate commented 7 months ago

Not sure if this is more a feature request or a bug based on the behavior.

Slackord version (please complete the following information):

Describe the bug

If Slackord crashes in the middle of an import, continuing an import later after relaunching Slackord will result in Slackord creating another duplicate Slackord Import category and set of channels. Discord doesn't yet support the ability to merge channels, so this means that there would be duplicate channels containing messages where the other one leaves off.

Ideally, I think there should be an option (and perhaps even a default option) to use the existing Slackord Import category and channels if they already exist.

To Reproduce

Steps to reproduce the behavior:

  1. Setup Slackord
  2. Select JSON folder, import JSON
  3. Type /slackord in Discord
  4. Force close the Slackord app in the middle of an import
  5. Re-open Slackord app, Connect, Select JSON / import JSON
  6. Type /slackord in Discord
  7. See Slackord create an entirely new Slackord Import category

Additional context

Since more than one import may be needed, especially with large imports (or perhaps subsequent imports), being able to use existing channels within a Slackord Import category would be really helpful, and would mean less duplicate channels.

thomasloupe commented 7 months ago

Hey @ScriptAutomate,

Thanks for the suggestion. My immediate reaction is to figure out why it crashed in the first place and eliminate the necessity to reimport all together, but I understand things 'just crash sometimes'.

I would first ask you if you could explain the crash if there was one, and what the error is/was. Additionally, would it be more useful to allow you to choose between a single channel import versus an all-or-nothing import?

Thanks!

thomasloupe commented 7 months ago

I also see no reason we cannot add the functionality to check if the Slackord Import category is/isn't full and remove, then re-create a new channel for a previously-failed import.

ScriptAutomate commented 7 months ago

Thanks for the quick responses!

Thanks for the suggestion. My immediate reaction is to figure out why it crashed in the first place and eliminate the necessity to reimport all together, but I understand things 'just crash sometimes'.

I would first ask you if you could explain the crash if there was one, and what the error is/was.

I'm not sure why it is crashing. It's a multi-day import that when I've noticed messages have stopped coming into Discord, I've gone to see the application and it is closed out and not running. When I re-launch, it has a fresh blank log. Is there a place I can look to get more details around why it may have crashed?

Additionally, would it be more useful to allow you to choose between a single channel import versus an all-or-nothing import?

That would be super useful, actually. If Slackord is unable to post to an existing channel it created on a previous run, then the ability to select which channels to import would be super helpful. Though, I made some of my own scripts to modify my source JSON directory to no longer include the channels it already imported as a current workaround.

I also see no reason we cannot add the functionality to check if the Slackord Import category is/isn't full and remove, then re-create a new channel for a previously-failed import.

What is still difficult, though, is that I realize I will have to do a subsequent import based on a new, much shorter time window and it would be more beneficial if Slackord could post to created channels under Slackord Import category that already exist, allowing for cumulative imports (in case Slackord crashes, or in case I want to import smaller chunks, or in case there is a future export that includes an import starting after where the latest import ends, etc.). Recreating/replacing an existing channel in a Slackord Import would be less ideal.

thomasloupe commented 7 months ago

That's pretty much grounds for an entire rework of the UI in a way where instead of processing all the data, we can selectively choose which channels we want to process. It sounds like it could have been due to resource exhaustion, as I'm not aware of any unhandled crashes where the application would just close, but I'm certainly not ruling it out! There's no way to get useful data from a hard crash like that.

However, if you're confident enough to download the source and run it directly from there, you would absolutely get some indication of what happened. I would highly recommend that. The issue with hard crashes means we don't really have any concept of what message we were on at the time of the crash. We would need to re-import everything and start over, then use some kind of outside message tracking to clear previously sent messages, and resume where we left off. I'm not against this, but it's a lot of work to get that kind of functionality.

So for now, I am looking for immediate ways to help resolve the issue, which isn't exhaustive, and does include the idea of choosing between single/multiple channels to import. I believe this functionality should be there by default, as Slackord wasn't designed with absolutely massive servers in mind, though it should have been. I think I can get the functionality for a single channel import in with little work.

ScriptAutomate commented 7 months ago

Got it, I see. Would a better feature request seem easier to achieve if the request was:

This way, Slackord can function as it already currently does when it comes to processing all the data, importing everything, etc. where the only difference would be not to create new channels in Discord.

If so, that would be something that could achieve what I'm looking for, because then I can manually clean up the JSON dirs prior to being imported into Slackord to start where I see the last messages left off. If that makes sense? It would allow for subsequent imports, and also allow for users to pickup where a crash may have left off (without needing Slackord to intelligently track and be aware of where it left off itself).

thomasloupe commented 7 months ago

Not quite.

I'm absolutely certain that you could personally clean up the JSON, but it's a lot more complicated. Here's an explanation:

When you attempt to clean up the JSON to pick up where you left off, you're going to be missing any parent thread that was created. Any posts to a previously created thread (which are now gone) are going to end up being completely null because Slackord won't have the thread parent ID to attach posts to. If we are restarting an import because a crash happened, the thread ID's, messages, etc are all lost anyway. There are some other issues that are similar the one I've just described that cause problems. Additionally, while I recognize you're likely totally capable of making these adjustments, I have to present code that makes that process simplistic for folks who haven't the understanding to make such manual changes. I think there are multiple issues we must look at here, based on severity to tackle all the issues here as best as possible:

  1. Solve the crash. - This eliminates 100% of the issue. If you have the time to work with me I can help you set up your own environment with the code to test if you're not familiar or need assistance. If you're interested, please reach out to me on our community Discord. We can figure out what's causing the crash and I can try to fix it.
  2. Allow selection between a Full Slack import, or just a single Channel import - I've just finished the work on this, and it's slated for the next release of Slackord. image
  3. If we fail to post messages to a specific channel, there is no reason to create a new Slackord Import category. The catch for this is that if we recreate the same channel name, we're not guaranteed the name is exactly the same, but I need to verify this, as cloning and creating are different. It's definitely doable though!

How does this approach sound?

ScriptAutomate commented 7 months ago
  1. Solve the crash. - This eliminates 100% of the issue. If you have the time to work with me I can help you set up your own environment with the code to test if you're not familiar or need assistance. If you're interested, please reach out to me on our community Discord. We can figure out what's causing the crash and I can try to fix it.

I'll join the Discord, and potentially reach out if I'm able to get time to set it up in this fashion for a useful crash log.

  1. Allow selection between a Full Slack import, or just a single Channel import - I've just finished the work on this, and it's slated for the next release of Slackord.

This would be super useful in the short-term, especially since the cause of the crash is unknown currently (and I'm not sure if there are multiple causes, different cause between each crash). Looking forward to this feature.

  1. If we fail to post messages to a specific channel, there is no reason to create a new Slackord Import category. The catch for this is that if we recreate the same channel name, we're not guaranteed the name is exactly the same, but I need to verify this, as cloning and creating are different. It's definitely doable though!

That would be awesome. I think it would be helpful if in the future if multiple exports are done in order to have smaller chunks of cumulative imports that don't each create their own subset of Slackord Import categories and channels of the same name.

Thank you for all of your responses and discussion on this!

thomasloupe commented 7 months ago

I have added functionality for single channel import. I have also added functionality to check if the Slackord Category category exists. If it does, we'll check the channel name and if it matches, we'll increment and append the incremented number to the end of the channel with a dash prepending it.

This should address your requests. If you run into another crash, let's focus on that crash in a newly-created issue if you don't mind. :) I'll go ahead and close this off, as the beta branch has been updated. Full release soon!