'Invisible' Editor for file undefined error during Notebook merger

[dkey-pugh]

When a user tries to merge two notebooks using the Advanced menu, on generating the diff, the console displays an error. This is not reflected in the UI and does not appear to affect the merger (i.e. the user is unaware that it happens if the console is not open).

The error is "Editor for file undefined and model $model not found!" where is an integer that appears to increment by two in each session each time the error is displayed.

[dkey-pugh]

Expanding the error gives:

provideCodeLenses	@	merger_view.js:567
	(anonymous)	@	editor.main.js:31
	(anonymous)	@	editor.main.js:31
	n.Class.derive._oncancel	@	editor.main.js:31
	t.asWinJsPromise	@	editor.main.js:31
	(anonymous)	@	editor.main.js:31
	c	@	editor.main.js:31
	(anonymous)	@	editor.main.js:31
	e.onTimeout	@	editor.main.js:31
	setTimeout (async)
	e.schedule	@	editor.main.js:31
	(anonymous)	@	editor.main.js:31
	e.invoke	@	editor.main.js:31
	e.fire	@	editor.main.js:31
	(anonymous)	@	editor.main.js:31
	o	@	editor.main.js:31
	t._emitEvents	@	editor.main.js:31
	e.emit	@	editor.main.js:31
	e._emitContentChanged2	@	editor.main.js:31
	e.setValueFromTextSource	@	editor.main.js:31
	e.setValue	@	editor.main.js:31
	updateReviewDecorations	@	merger_view.js:528
	init	@	merger_view.js:363
	setTimeout (async)
	setTransitionTimeout	@	merger_view.js:36
	_model.on_file_diff_complete.attach	@	merger_view.js:367
	notify	@	event.js:16
	_.each	@	merger_model.js:249
	.each..forEach	@	underscore.js:79
	update_compare_details	@	merger_model.js:245
	get_notebook_func.call.then	@	merger_model.js:196
	tryCatcher	@	bluebird.js:4409
	Promise._settlePromiseFromHandler	@	bluebird.js:2560
	Promise._settlePromiseAt	@	bluebird.js:2636
	Promise._settlePromises	@	bluebird.js:2752
	Async._drainQueue	@	bluebird.js:131
	Async._drainQueues	@	bluebird.js:141
	Async.drainQueues	@	bluebird.js:66
	attributes (async)
	(anonymous)	@	bluebird.js:3749
	Async._queueTick	@	bluebird.js:149
	Async.settlePromises	@	bluebird.js:124
	Promise._queueSettlePromises	@	bluebird.js:2692
	Promise._fulfillUnchecked	@	bluebird.js:2707
	Promise._fulfill	@	bluebird.js:2604
	(anonymous)	@	bluebird.js:3000
	(anonymous)	@	rserve.js:1241
	callback	@	rserve.js:1116
	socket.onmessage	@	rserve.js:995

[gordonwoodhull]

This only happens on the second or third time through the dialog, e.g. if we previously hit #2634 or on trying to merge a second (different) notebook. I think it's likely that monaco editors are still alive after the dialog closes, and it's trying to set them up with codelens, but they aren't in the notebook.

[gordonwoodhull]

I was incorrect.. it doesn't happen every time, but it can happen on the first try.

[gordonwoodhull]

I am wondering if the other editors are in the tab which we removed, showing the side-by-side diff.

I don't like to just comment out an error, but that's the best I can think of for now.

[gordonwoodhull]

I'm restoring the error as a warning instead. Something is still wrong here but I can't tell if it's related to #2639

[jameesy]

Hopefully this screenshot helps visualise what is happening. For context:

• The first console.log is the this._editors object.
• Then a loadash _.filter call which filters over the above object looking the model_id and stores it as a variable model_filename.
• The next console.log is the model_filename variable discovered above.

As we can see, it cannot find $model2 so, in this instance, part1.md is never returned. It then for whatever reason increments by two and comes back for a second look and finds $model4 in the Object and then returns correctly.

I closed and went through the merge dialog again and the same thing happened but the numbers had incremented yet again to 6 and 8.

I'm unsure as to why this is happening, and why it is happening sporadically for myself. I have had a few occasions where the first model_id is $model2 and it doesn't throw an error, but still runs the function twice.

[gordonwoodhull]

Yes I'm seeing the same thing. I agree that it is probably not related to #2639 and is not affected by whether it's the first merge or not.

I'm not as concerned about this issue since everything seems functional otherwise. That's why I changed it to a warning. It is still concerning because obviously the code is not working as expected.

This can be put off for 2.2 or whenever. Likely it will manifest as another bug!