[patch][cla][pr] dojo.js guardCheckComplete has try / finally without catch, breaking IE8 compatibility

[Nathaniel Mills]

The release notes in 1.8.1 talk about IE8 compatibility, but my application breaks as soon as dojo.js is loaded because it has a try / finally without a catch in the guardCheckComplete method. The fix is simply to add an empty catch as shown below highlighted by add --> bold:

		guardCheckComplete = function(proc){
			try{
				checkCompleteGuard++;
				proc();
add --> 		}catch(e){
			}finally{
				checkCompleteGuard--;
			}
			if(execComplete()){
				signal("idle", []);
			}
		},

I looked in the 1.8.3 and latest dojo.js source and it also has this problem.

[Nathaniel Mills]

I think to preserve the current function, the change should actually throw the caught error as in: add --> } catch(e){throw e; } finally

[Nathaniel Mills]

ready.js has the same issue with a try/finally missing the catch

[bill]

IE8 works fine for me, and as far as I know, no one else has this problem that you mention.

How do you reproduce it?

[Nathaniel Mills]

I tripped across this because html fragments attempted to be brought into the js file using dojo.text! were not being retrieved so the html was null and caused the exception in guardCheckComplete. When I searched the web, I found several references to IE8 not supporting try / finally without catch. One example is here: ​http://stackoverflow.com/questions/5023674/jquery-throws-ie-specific-error If you search on try finally ie8 you can see other references to this problem. Another reference is here: ​http://stackoverflow.com/questions/8846471/have-you-had-problems-with-dojo-1-7-1-in-internet-explorer-7-8 I realize that the real problem is our own code causing the exception to occur, but the result is misleading because IE8 debugger stops on the error (try without a catch) and we can't see the exception that occurred. It would be better to catch the exception and throw it so it bubbles up to help people understand the problem they have created.

When I tried to reproduce today by creating an error in the html, I'm getting a different error: Expected ":" just past the word finally when the code was commented:

	guardCheckComplete = function(proc){
		try{
			checkCompleteGuard++;
			proc();
         	// } catch(e) {throw e;
	        } finally { // wnm3: fixed for IE8
			checkCompleteGuard--;
		}
		if(execComplete()){
			signal("idle", []);
		}
	},

To reproduce, you have to have something that triggers an exception to be thrown in the guardCheckComplete method. Unfortunately, my code has devolved to the point where I can't easily get it back to the original situation that brought this error to light. What I did today was just cause an error in the html by referencing a non-existing dijit class and commented out the catch clause.

[bill]

I see. Although I worry that adding the catch(e) workaround will make debugging harder on other platforms, especially when "pause on uncaught exceptions" (only) is set, because then won't chrome only stop on the second throw, rather than the original one?

[Nathaniel Mills]

Thanks Bill. Please note that I'd updated the suggested fix to include a throw e; statement after catching the exception so the exception would still exist and the break on uncaught exceptions would be triggered as I don't believe the method is called in an outer try/catch environment.

The add line should read: } catch (e) { throw e; } finally { ...

[bill]

Yes, I understand that. My comment still stands.

[Nathaniel Mills]

Yes, it would stop on the second throw, but the exception content remains the same.

[split]

I have simular issue on IE8. I created a custom widget, and then require[<my widget>]. But function guardCheckComplete will always throw an exception every "first" time on the page.

However, it works fine if I try require[<my widget>] again on the same web page. Please see my attachment.

​https://bugs.dojotoolkit.org/attachment/ticket/16617/dojo_require_imcompatiable_ie8.zip

[split]

BTW, I found that the root cause is dojo.connect. If the old-style dojo.connect() is used in my widget, dojo.js will throw exception during requiring.

[dylan]

Makes sense, since you're not pulling in that dependency (if you pulled in "dojo" as a dependency that would have worked, though that's not really what you want to do anyway)...

[barney]

Hi guys, inherited an old site with a commitment to support IE8 and whose codebase depends heavily on dojo 1.9.1. I'm having difficulty understanding the rationale in closing this – reading between the lines I'm assuming some variation of "user error/won't fix/not a bug". Although looking at the timestamps, it could well be "OP moved on in life".

@split do you remember solution?

[bill]

Hmm, it sounds like split's problem was that he didn't specify his dependencies correctly. But AFAICT the root issue still stands, so I guess we should reopen this.

[dylan]

PR needs review at ​https://github.com/dojo/dojo/pull/199 .

[Rawld Gill]

​https://github.com/dojo/dojo/pull/199 looks good to me.

[Dylan Schiemann <dylan@…>]

In a275e823/dojo:

Error: Processor CommitTicketReference failed

Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

[Dylan Schiemann <dylan@…>]

In 37e8256/dojo:

Error: Processor CommitTicketReference failed

Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

[Dylan Schiemann <dylan@…>]

In 430df9b/dojo:

Error: Processor CommitTicketReference failed

Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

[Dylan Schiemann <dylan@…>]

In 16686d6/dojo:

Error: Processor CommitTicketReference failed

Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled?