Exception in Entry Editor (Braces don't match)

[erelsgl]

• [ X ] I have tested the latest master version from http://builds.jabref.org/master/ and the problem persists

Version:

JabRef 4.1
Linux 4.4.0-104-generic amd64
Java 1.8.0_161

Steps to reproduce:

1. Open an entry with a title with upper case letters but no braces.
2. Add braces and save.

18:15:58.228 [pool-4-thread-1] ERROR org.jabref.logic.autosaveandbackup.BackupManager - Error while saving file. org.jabref.logic.exporter.SaveException: Error in field 'title': Braces don't match. at org.jabref.logic.exporter.BibtexDatabaseWriter.writeEntry(BibtexDatabaseWriter.java:169) ~[JabRef-4.1.jar:?] at org.jabref.logic.exporter.BibDatabaseWriter.savePartOfDatabase(BibDatabaseWriter.java:198) ~[JabRef-4.1.jar:?] at org.jabref.logic.exporter.BibDatabaseWriter.saveDatabase(BibDatabaseWriter.java:150) ~[JabRef-4.1.jar:?] at org.jabref.logic.autosaveandbackup.BackupManager.performBackup(BackupManager.java:123) ~[JabRef-4.1.jar:?] at java.util.Optional.ifPresent(Optional.java:159) ~[?:1.8.0_161] at org.jabref.logic.autosaveandbackup.BackupManager.lambda$new$0(BackupManager.java:49) ~[JabRef-4.1.jar:?] at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511) [?:1.8.0_161] at java.util.concurrent.FutureTask.run(FutureTask.java:266) [?:1.8.0_161] at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) [?:1.8.0_161] at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) [?:1.8.0_161] at java.lang.Thread.run(Thread.java:748) [?:1.8.0_161] File: aboutdialog.fxml not found, attempting with camel case

[lenhard]

Thanks for reporting. This is the autosave kicking in that is trying to save the file while the braces are still incomplete.

What should be changed here? Should anything be changed? The exception doesn't break anything, JabRef continues to work. And we can't exactly stop the autosave from attempting to save.

[erelsgl]

I think the exception should not be shown in this case. It is confusing and might hide other issues (for example, if JabRef hangs and I try to find the reason, I naturally look at the list of exceptions, and having an unrelated exception is distracting).

[lenhard]

Yes I understand and I generally agree, but how should this go about?

The autosave saves everything immediately. After adding an opening brace and trying to save, JabRef cannot just swallow the error, because it does not know if the user will ever add a closing brace. And if the user does not do that, then the error message is valid and important.

As far as I understand, the solution to your problem is probably to disable autosave.

[Siedlerchr]

We could trigger autosave on field focus change, e.g. When the user leaves the field
Don't we already do this with the Coarse Change Event?

[tobiasdiez]

What about replacing

jabref/src/main/java/org/jabref/logic/autosaveandbackup/BackupManager.java

Lines 125 to 127 in 2cdf085

	} catch (SaveException e) {
	LOGGER.error("Error while saving file.", e);
	}

with

        } catch (SaveException e) {
            LOGGER.error("Error while saving file: " + e.getLocalizedMessage());
        }

In this way, the stacktrace is not displayed (which is unimportant for these kind of exceptions that are expected to happen to some extend).

[lenhard]

@Siedlerchr The problem in this is that you tie the correctness of the autosave to some future event, which may never be triggered due to exceptional circumstances or could easily be swallowed somewhere else. And you tie the autosave logic to gui events. I am very much in favor of the autosave doing the local decision based on entry events only.

I really like the suggestion by @tobiasdiez

[erelsgl]

But why show a message at the console at all? Why not just mark the erroneous field with a warning sign (as it is done now)?

[lenhard]

@erelsgl There can be cases where the save fails that is not visible in the entry editor. An example would be that the file to save to is on a network drive that has disappeared in the meantime. In that case, it is good to have a message on the console.

I proposed a PR #3865 that skips reporting any invalid field errors during autosave. They no longer make it into the log, but the warning sign stays there. A regular save operation will still trigger the warning dialog.