Skip to content
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

Beautify On Save crashes Atom editor #924

Closed
3 tasks done
phumberdroz opened this issue Apr 16, 2016 · 45 comments
Closed
3 tasks done

Beautify On Save crashes Atom editor #924

phumberdroz opened this issue Apr 16, 2016 · 45 comments

Comments

@phumberdroz
Copy link
Contributor

Description

Atom crashes with the code below if Beautify On Save is enabled

Expected Results

Atom should just save the file. :)

/**
 * Created by Pierre on 05.04.16.
 */
var express = require("express");
var router = express.Router({
    mergeParams: true
});
var Customer = require("../../models/customers");
var middleware = require("../../middleware/index.js");
//TODO add a way for pagination
//INDEX - show all Customers
router.get("/", function(req, res) {
  var regexid = /^[a-f\d]{24}$/i;
  var search = req.query.search;

  if (regexid.exec(search) !== null) {
    Customer.findById(search, function (err, foundCustomer) {
    if (err) {
        console.log(err);
    } else {
        res.end(JSON.stringify([foundCustomer]));
    }
});
  } else {
    var re = new RegExp(search);
    var offset = 0;
    if (req.query.offset) {
        offset = req.query.offset;
    }
    Customer.find().or([{
        'name': {
            $regex: re
        }
    }, {
        'mail': {
            $regex: re
        }
    }, {
        'phone': {
            $regex: re
        }
    }, {
        'customernumber': {
            $regex: re
        }
    }, {
        'street': {
            $regex: re
        }
    }]).skip(parseInt(offset)).limit(20).exec(function(err, allCustomers) {
        if (err) {
            console.log(err);
        } else if (allCustomers.length === 0) {
            res.sendStatus(404);
        } else {
            res.end(JSON.stringify(allCustomers));
        }
    });
  }


});

module.exports = router;

Error happens only if this part is in but this part alone does not cause the error to happen

var regexid = /^[a-f\d]{24}$/i;
  var search = req.query.search;

  if (regexid.exec(search) !== null) {
    Customer.findById(search, function (err, foundCustomer) {
    if (err) {
        console.log(err);
    } else {
        res.end(JSON.stringify([foundCustomer]));
    }
});
  } 

Steps to Reproduce

  1. Add code to Atom editor
  2. Run command Save with CMD + S
  3. Atom Crashes

Debug

https://gist.github.com/meat147/982e1528972e4e6c234340feade905fa

Checklist

  • I have tried uninstalling and reinstalling Atom Beautify to ensure it installed properly
  • Searched for existing Atom Beautify Issues at https://github.com/Glavin001/atom-beautify/issues
    so I know this is not a duplicate issue
  • Generated debugging information and added link for debug.md Gist to this issue
@prettydiff
Copy link
Collaborator

Does not appear to crash jsbeautifier.org

@victorlambert
Copy link

victorlambert commented Apr 16, 2016

Same problem here since atom update 1.7.1. Atom crash and replace my file I was trying to save by a blank zero byte file ... very annoying.

I can reproduced this bug on every file, juste by saving several times in a row.

@prettydiff
Copy link
Collaborator

@victorlambert When this problem occurs is it any kind of language or a particular language that causes the problem? Any kind of evidences as to whether this is a language specific, Atom Beautify specific, or Atom defect would be helpful.

@victorlambert
Copy link

victorlambert commented Apr 16, 2016

Seems to be all languages I use, I just tried JSON, JS, HTML and it always crashed when atom beautify had to indent code. I noticed that saving is now a little bit longer, like atom is freezing when I manage to not make it crash. My workaround for now is just to disable "beautify on save" and to save and beautify separately.

Since this is very annoying for me, I would provide any information I can, if anyone could tell me where I can find log files.

@phumberdroz
Copy link
Contributor Author

So for me it happens only with that one file. If I press cmd+s it crashes.

@phumberdroz
Copy link
Contributor Author

I forgot to add that if atom crashes the file does not get saved instead it gets deleted!

@Glavin001 Glavin001 self-assigned this Apr 16, 2016
@Glavin001
Copy link
Owner

@victorlambert : You can create the debugging log data by following the instructions found at https://github.com/Glavin001/atom-beautify/blob/master/ISSUE_TEMPLATE.md#how-to-create-debugmd-gist

I tried @meat147 's code above and am unable to reproduce this problem -- it beautifies properly and saves without crashing.

Also, looking at the debugging logs everything appears to be working.

I do notice that the first time I save after recently opening Atom it takes a little longer, which I think is because Atom-Beautify and other packages that handle the core:save and core:save-as events are activating and loading. See #876 (which I may have to re-open now).

It would be helpful if someone who can reproduce this issue could open up Developer Tools and step through the code to see where it gets stuck and crashes Atom. Any other information that will help us reproduce would be great, too. Thanks! Hopefully we can solve this soon.

@phumberdroz
Copy link
Contributor Author

I uploaded two Videos like i said it happens only if i press cmd+s and I am on Mac

https://youtu.be/dAP-ahteu-o
https://youtu.be/F8RlmgkhT_w

And if I beautify with the cmd+shift+p and then beautify works just fine.
Only if beautify on save is on and also only if i press cmd+s

@joecarney
Copy link

Am having the same problem. This package crashes Atom randomly on save (any type of file, although I'm mostly in .js & .jsx). What is even worse, is that it creates a zero length file deleting everything that you are currently editing in the open file. Fortunately I had Git to save the day. Have disabled this package until a fix can be found. Have tried debugging but cannot find the cause :(

@phumberdroz
Copy link
Contributor Author

@joecarney disabling on save should stop this from happening at least it did for me

@joecarney
Copy link

@meat147 just tried disabling all beautify on save and it has crashed again :(

I have developer tools open, with preserve log enabled, when it crashes the inspected target gets disconnected and when I reload there is nothing in log from beautify. Am happy to help debugging this package but no idea as to the best way of capturing anything at this stage. I very much doubt creating a Gist of my code will help, it crashes in many different files

Will leave this package disabled until a fix has been found.

@Glavin001
Copy link
Owner

Unfortunately, I'm in the middle of final exams and writing my thesis this week, so I am not able to allocate much time to debugging this right now.
I may have some time to investigate this further after April 22nd, and more time after April 27th and before beginning of May.

In the meantime, I recommend that each of you experiencing this issue disable the beautify on save temporarily.
I do not think it is necessary that you disable the Atom-Beautify package entirely, as only the beautify on save functionality is having issues and beautifying normal appears to be reported as working.

Note: With this issue I am focusing on the users that are experiencing Atom crashing after beautifying on save. I recognize that the first time you save it takes a moment to load Atom-Beautify. On slower computers, I can imagine this delay becomes more noticeable. I will also be working on the performance with #876, so please subscribe to that issue there.


I have just published v0.29.3 release.
I cleaned up debugging and added a lot more verbose debugging to the beautification code that handles on save events.

I recommend that each of you do the following steps listed below. If you follow the steps it will likely be very obvious where things start to go wrong and Atom freezes and consequently crashes.

  1. Uninstall Atom-Beautify.
  • Ensure we start fresh.
  1. Reinstall Atom-Beautify.
  • This will install a fresh copy of the latest dependencies, such as JS-Beautify and Pretty Diff (which are used for beautifying JavaScript, HTML, JSON, and much more).
  1. Restart Atom
  • Ensure the Node.js module cache is clean. This causes problems sometimes when installing or updating Atom packages.
  1. Enable verbose logging in Atom-Beautify.
  • As @joecarney mentioned, "there is nothing in log from beautify", and this is because by default Atom-Beautify only logs warnings and errors. You want to go to Atom-Beautify's package settings -> General group -> Logger Level -> Change value to verbose:
    image
  1. Enable Beautify on Save for your desired language.
  • Go to Atom-Beautify's package settings -> JavaScript (or any other language) -> Toggle on Beautify On Save option:
    image
  1. Open Developer Tools.
  • Atom -> View -> Developer -> Toggle Developer Tools.
    image
  1. Start recording your screen.
  • I have found that sometimes it is easiest to simply record your screen so you can see the quick events that happen before Atom crashes. In this case, we are watching the developer console logs until Atom crashes and they cannot be seen anymore.
  • For Mac users, you could use the built-in Quicktime: https://support.apple.com/en-ca/HT201066#screen
  1. In Atom, save your file and record the logs from the developer console log out from Atom-Beautify.

  2. The objective is to find what code is causing Atom to crash.

  1. Let me know anything you learn!
  • Of course, you could add breakpoints and step through the code, however these steps may end up being quicker as the logs should show us a fairly precise location of the code that eventually causes Atom's freezing and crashing.

Given my very limited time this month, this is the best I can do for now. I hope we can figure this out together! Thanks for your patience and feedback.

@emveeoh
Copy link

emveeoh commented Apr 17, 2016

I noticed this mostly when using the JSON beautifier. It will consistently crash with 'Beautify On Save' enabled. Additionally, the file is zero'd out and all contents are lost. Disabling 'Beautify On Save' and running the beautify manually works as expected with no crash or zero'd data.

Atom 1.7.1
atom-beautify 0.29.4

@Glavin001
Copy link
Owner

@emveeoh : What do you see in the developer console logs when you follow the steps provided in the comment #924 (comment) ? It is critical that we narrow down what lines of code are causing Atom to crash.

Additionally, the file is zero'd out and all contents are lost.

I've heard of this happening when users beautify a few multiple times before beautification has completed, such as saving quickly and repeatedly. See #481 (comment) for an old comment that explains some more. Is this what causes it for you? Or it will delete contents even if you only trigger beautification once?

@emveeoh
Copy link

emveeoh commented Apr 17, 2016

@Glavin001

I've heard of this happening when users beautify a few multiple times before beautification has completed, such as saving quickly and repeatedly. See #481 (comment) for an old comment that explains some more. Is this what causes it for you? Or it will delete contents even if you only trigger beautification once?

It deletes contents nearly 100% of the time.

I am also now seeing this same behavior with .ejs files (Javascript templates). I tried to change the document type from 'Javascript template' to 'html' at the bottom of the screen. I did a 'save' and Atom crashed and the page contents were lost. Unselected 'Beautify On Save' for every language and I have not had any problems since.

I initially assumed it was just the JSON beautifier. Now, I think it's any of them that execute automatically on save.

I will follow the steps in comment #924 and report back to you shortly.

@emveeoh
Copy link

emveeoh commented Apr 17, 2016

@Glavin001
I was unable to find any errors with the dev tools... Atom crashes before anything useful is logged. However, I was able to film the crashes for you to see. If there is anything else I can do to help you debug, please let me know. I will be happy to do so.

https://youtu.be/ces74eQKBFk

@phumberdroz
Copy link
Contributor Author

@emveeoh your video is set to privat you need to put it to unlisted.

@emveeoh
Copy link

emveeoh commented Apr 17, 2016

@meat147 @Glavin001 Oops! Sorry guys... Video should be public now... Apologies.

@Glavin001
Copy link
Owner

So at https://youtu.be/ces74eQKBFk?t=4m23s
it has logged beautifyFilePath completionFun from https://github.com/Glavin001/atom-beautify/blob/master/src/beautify.coffee#L213
I think somewhere around https://github.com/Glavin001/atom-beautify/blob/master/src/beautify.coffee#L222-L225 is causing problems since https://github.com/Glavin001/atom-beautify/blob/master/src/beautify.coffee#L191-L195 is not reached and logged.
If someone could debug this (add breakpoints, and ensure this is the path taken) that would be helpful.

Thank you @emveeoh for your video! This helped me understand what it is looking like for you users who are experiencing this 😃. Now to debug and fix it.

@Glavin001 Glavin001 changed the title my file crashes Atom with Beautify enabled Beautify On Save crashes Atom editor Apr 19, 2016
@Glavin001
Copy link
Owner

/cc @janwerkhoven you should subscribe to this issue.
Hopefully we can resolve this soon.

@Glavin001
Copy link
Owner

Published a patch to v0.29.5.

I am keeping this issue open until I have confirmation it resolves the issues described above.
Thank you everyone for your patience, feedback, and assistance with diagnosing this issue.

Now, I really need to get back to writing my thesis 😜 ! Only 1 more day to go.
I hope there won't be any more critical issues for at least a day or two so I can finish off this semester of school.

@emveeoh
Copy link

emveeoh commented Apr 19, 2016

Awesome! Thanks @Glavin001 ... I will test and report back.

@emveeoh
Copy link

emveeoh commented Apr 19, 2016

Serious problem with this new version @Glavin001.... Seems to be looping the save command.... Making video now...

@Glavin001
Copy link
Owner

Shoot! I know what is going on. For some reason it stopped looping when I tested everything.
If you could provide a sample of your code that is causing this, it will help me test this more thoroughly. Thanks.

@emveeoh
Copy link

emveeoh commented Apr 19, 2016

@Glavin001 You might choose to push a version that disables 'Beautify On Save' altogether until you have time to resolve the issue...???

@epelc
Copy link

epelc commented Apr 19, 2016

I just tested the new version and it randomly deletes all of the file contents like before. But it is no longer crashing atom.

Also I held down ctrl+s on for a few seconds on a different file while attempting to trigger the bug. But I think it started looping the save(even a minute or two after I let go) like @emveeoh mentioned. The editor became extremly slow to do anything. Then I closed the file and got this error.
image

First stack trace: https://gist.github.com/epelc/b004422105274bbcdd5f9dc1b8556a23
Second stack trace: https://gist.github.com/epelc/36a4809a9fd4f11a806b47e518e75364

Atom: 1.7.2
OS: Windows 10 64bit

@emveeoh
Copy link

emveeoh commented Apr 19, 2016

@Glavin001 Video uploading now... https://youtu.be/Nl6L-2nZ91g

@epelc
Copy link

epelc commented Apr 19, 2016

@Glavin001 best bet might be to rollback the recent change(I'm not sure if it causes issues in 1.6.2) and add a warning that 1.7.x is not supported yet.

@Glavin001
Copy link
Owner

@emveeoh I see why it loops in yours and not in mine. Your Atom is adding a newline at the end of the file after each beautification which is then removed on beautification, and it loops until beautification has no changes to be saved.

Working on a fix right now.

@Glavin001
Copy link
Owner

Got a fix. Tested and working with the option Ensure Single Trailing Newline enabled for Whitespace package.
Publishing soon.

@Glavin001
Copy link
Owner

Published a patch for the infinite loop at v0.29.6

Tested and working with the option Ensure Single Trailing Newline enabled for Whitespace package, as shown to cause infinite loop in @emveeoh 's video.

@Glavin001 Glavin001 reopened this Apr 19, 2016
@emveeoh
Copy link

emveeoh commented Apr 19, 2016

Makes me wonder if this was the problem all along... Maybe, the original fs.write was working? Hmmm....

@Glavin001
Copy link
Owner

Makes me wonder if this was the problem all along... Maybe, the original fs.write was working? Hmmm....

No, the infinite loop was my fault with the recent push of v0.29.5.

I'm trying to recall why I even used fs.write and I think it may have been because I did not want to create an infinite loop of saving. The new implementation of beautify on save uses the traditional beautify function that you get when you run command Atom-Beautify: Beautify Editor, which changes the TextEditor.

The reason why this is this complicated in the first place is because of how Atom deals with saving. Atom-Beautify is asynchronous, beautifiers do not block Atom and can run in the background and can return later if necessary. Atom however expects the changes to the TextEditor, triggered by editor.onWillSave, to be synchronous. So the changes must have been applied before the File System write actually occurs. Of course, Atom-Beautify is not done beautifying when this happens.

Anyway, hopefully it is all working now! The only issue with the current implementation is #935.

@Glavin001
Copy link
Owner

Please post here to confirm that it is working for you. So I can sleep easy tonight 😜. Thank you.

@epelc
Copy link

epelc commented Apr 19, 2016

@Glavin001 working besides for the problem in #935 but that is minor.

Maybe a fix for #935 and possibly the lag after multiple saves could be to cancel running beatifications when you save again and when you close a file. I haven't done anything with atom plugins so this may not work at all. But just a thought.

@emveeoh
Copy link

emveeoh commented Apr 19, 2016

Great job @Glavin001!! Works as expected.... Tested: JSON, LESS, SASS, SCSS, HTML CSS ... Smooth sailing... Best of luck on your exams/thesis!

@herrherrmann
Copy link

Works for me, too. Thanks so much for the spontaneous fix and good luck with your thesis.

@victorlambert
Copy link

All good ! 👍 Thank you for your reactivity and good luck !

@beezenees
Copy link

beezenees commented Apr 21, 2016

Update seems to work for me for JS & XML - However, my TSS ended up crashing with the same issue. I am on the updated version of Atom Beautify. If there is a need for me to give additional info, please let me know.

@prettydiff
Copy link
Collaborator

@pinesy Could you please provide the generated debug.md file as a public gist, if possible? If that is not possible could you post a sample of the TSS code so that somebody can reproduce the problem?

@phumberdroz
Copy link
Contributor Author

@Glavin001 I think this issue can be closed.. For me it is working perfectly fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants