13

I met this Breeze error

[Illegal construction - use 'or' to combine checks]

on Chrome when loading the edit page of an entity. When I refresh the page, the error message no longer appears. This error happens randomly, irregularly on my site. I could not reproduce it using a specified scenario, just met it by random.

I see this error message inside Breeze code

if (curContext.prevContext === null) {
    curContext.prevContext = context;
    // just update the prevContext but don't change the curContext.
    return that;
} else if (context.prevContext === null) {
    context.prevContext = that._context;
} else {
    throw new Error("Illegal construction - use 'or' to combine checks");
}

Could you please tell me: based on above block of code, in which cases this error is thrown?

Thanks a lot.

Tom Fenech
  • 72,334
  • 12
  • 107
  • 141
zorro-lp
  • 147
  • 1
  • 8
  • +1! Just got this error as well – GETah Feb 23 '14 at 22:57
  • 2
    We are getting this error too. Considering that we don't get the error in other browsers (i.e. IE), that we have not changed the version of breeze or the code that accesses it and that Google released a new version of Chrome late last week, I think this very well could be due to the new Chrome version. Maybe a bug in Chrome or breeze or something more subtle like an incompatibility or use of questionable/problematic javascript code. – steve Feb 24 '14 at 21:05
  • 1
    @steve This is definitely related to the latest Chrome update. The moment I upgraded from version 32 to 33, I started seeing this problem. – larspars Feb 25 '14 at 08:44
  • 1
    I can't seem to repro this on Chrome 33. Can someone email a simple test, hopefully using the DocCode framework/model in the Breeze zip, to breeze@ideablade.com? Or at least give me a little more context. – Jay Traband Feb 25 '14 at 18:13
  • 1
    @JayTraband it happens randomly unfortunately. I've seen it happening twice today. – GETah Feb 25 '14 at 18:16
  • +1, same error here :( – Pablo Romeo Feb 25 '14 at 18:36
  • 1
    Error occurs somewhat randomly. In our application, it may happen on one of the first few breeze queries -- but not necessarily the first. If it does not happen in the first few queries, then it does not happen for rest of the application session. It seems that there is a time window at application startup when the error can happen. And once it happens, all subsequent queries fail with the same error. – steve Feb 25 '14 at 21:47

6 Answers6

7

My team has been having this problem too. It started happening about a month ago, but has really increased in frequency over the past 1-2 weeks. Possibly the recent chrome release to blame.

Here is what I know, all commentary relative to breeze 1.4.1:

-The behavior is intermittent, and seemingly occurs at random. To me, this indicates a timing issue.

-The primary browser generating this error is chrome. We also support firefox and IE and have no concrete evidence that any browser but chrome is throwing this error. Perhaps the recent release of chrome has a different performance profile that exacerbates a pre-existing issue (again, timing?)

-For us, turning off bundling and minification seems to eliminate the problem. I don't believe there is an issue with our minified code (Microsoft Web Optimizations) as everything works on other browsers regardless. This to me again indicates a timing issue.

-Finally, I was just able to reproduce it in my dev environment with the chrome developer tools open. Using a q promise stack, and painfully navigating the minified code I was able to narrow it down to this: At my app start, I call fetchMetadata. Within the fetchMetadata success handler, I make a call to metadataStore.getEntityType('some_entity') and it is within this breeze method that the error is being generated in my scenario. Something with the metadata store isn't consistently initialized or setup at this early stage in the pages app lifecycle.

EDIT: From the comments, this appears to be a chrome 33 bug where null !== null at random times. For unknown reasons, the minifying of the breeze.debug.js file seems to be related (most/all reports of the problem are happening on a minified version of breeze). For me, changing the following code in breeze.debug.js:

} else if (context.prevContext === null) {
    context.prevContext = that._context;
} else {
    throw new Error("Illegal construction - use 'or' to combine checks");
}

to:

} else if (context.prevContext == null) {
    context.prevContext = that._context;
} else {
    throw new Error("Illegal construction - use 'or' to combine checks");
}

(change === to == on first line) seems to have resolved the issue as a workaround. All other aspects of breeze are working fine for me after this change.

One other thing I have noticed is that the minified version of the function has an argument with the same name of the function (t). This still doesn't explain the results of the "Aaarg" test.

   function t(n, t) {
        if (n._context) {
            for (var i = n._context; i.prevContext != null; )
                i = i.prevContext;
            if (i.prevContext === null)
                return i.prevContext = t, n;
            if (t.prevContext == null)
                t.prevContext = n._context;
            else
                throw new Error("Illegal construction - use 'or' to combine checks");
        }
        return b(n, t)
    }
mathias999us
  • 367
  • 1
  • 3
  • 13
  • 1
    @Ward: Here is what you get: ** Aaargh! 'curContext.prevContext': undefined 'context.prevContext': null – mathias999us Feb 25 '14 at 21:02
  • Here is another minor clue: I have a system set up to persist the metadata store in client storage for the duration of a session. When the local metadata is found on the page's app start, it uses that and calls entityManager.metadataStore.importMetadata() instead of calling entityManager.fetchMetadata. After calling importMetadata, it then calls the same function as the success handler for the fetchMetadata call, which in turn calls metadataStore.getEntityType(), and I have just reproduced the illegal construction error in this situation as well. – mathias999us Feb 25 '14 at 21:30
  • Apparently in chrome 33, sometimes null !== null. If you look at the values reported for the "Aaargh!" test, there is no way execution should have entered the block that throws the illegal construction error. Furthermore, I've found that the failure in my scenario happens on the second line of metadataStore.getEntityType: assertParam(okIfNotFound, "okIfNotFound").isBoolean().isOptional().check(false); Specifically, it happens on the isOptional call. There is no dependency that I can see on asynchronous code in the call stack from this point, ruling out a timing issue. – mathias999us Feb 25 '14 at 23:18
  • I have not been able to reproduce the problem after changing: } else if (context.prevContext === null) { to } else if (context.prevContext == null) { and everything else seems to be working fine. – mathias999us Feb 25 '14 at 23:21
  • are you minifying breeze or running unminified? My testing suggests that minification affects the behavior. Would be helpful to know whether your testing is consistent with mine. – steve Feb 25 '14 at 23:31
  • Steve - we have been minifying the breeze.debug.js file. I haven't been able to reproduce this when the minification is turned off, so that is consistent with your testing as well. – mathias999us Feb 25 '14 at 23:54
  • I cannot reproduce the bug, but so coincident, I only met this error with my production site (minified script), but not in my localhost (not minified). – zorro-lp Feb 26 '14 at 02:42
  • @mathias999us are you saying: you cannot reproduce the bug when adding [== null] criteria to the code? if (curContext.prevContext === null || curContext.prevContext == null) { curContext.prevContext = context; // just update the prevContext but don't change the curContext. return that; } else if (context.prevContext === null || context.prevContext == null) { context.prevContext = that._context; } else { throw new Error("Illegal construction - use 'or' to combine checks"); } – zorro-lp Feb 26 '14 at 02:44
  • @itjunkie - Yes, difficult to read in the unformatted comments, but after replacing the second === check with == (the check immediately preceding the "else illegal construction" block) I have not been able to reproduce the error, and all other aspects (excuse the pun) of breeze seem to be working fine for me. – mathias999us Feb 26 '14 at 15:03
  • I can't reproduce this locally even with the minified version. The issue does however occur when running in production. I think there is a timing factor in the equation... – GETah Feb 26 '14 at 18:11
  • @mathias999us - Thanks for the research. I am confused but will run our test suites with your 'fix' and see if everything else looks good. We are planning on a general release later this week anyway and I'll try to get this in with that. I'd really like to understand if this is a minification bug or if anyone has experienced this failure with the non minified version. – Jay Traband Feb 26 '14 at 19:58
6

We're kind of stumped because no one can pin down when this happens.

Would you all do us a favor and modify your breeze.debug.js to capture more information about the state of affairs when it throws?

Maybe you can add this:

} else {
     console.log("** Aaargh! 'curContext.prevContext': " + curContext.prevContext +
                " 'context.prevContext': " + context.prevContext);
     throw new Error("Illegal construction - use 'or' to combine checks");
}

Grasping at straws. All info helps.

Update 26 Feb 2014

AHA! Thank you @steve, @matthias, and others!

As I privately suspected, something, somewhere, has decide to set prevContext to undeclared instead of null. I was going to recommend that we switch to "==" anyway ... which would handle both cases. Falsiness is good enough IMO. We'll get back to you when we do it (assuming no one inside the Breeze team objects to applying a fix that we can't test).

Update 27 Feb 2014

I'm running my DocCode tests with breeze.min.js in Chrome v33 and they all pass. Frustrating. Jay will run his tests with breeze.min.js in Chrome v33 too ... and we will see if any of them fail for him. I am not hopeful.

I get the expected behavior for sensible (including illegal) variations of parm (undefined, null, true, false, a string) on the line from getEntityType that @Matthias mentioned

assertParam(parm, "okIfNotFound").isBoolean().isOptional().check(false);

My static analysis of the code (who trusts that?) tells me that the first comparison operator must remain === whereas the comparison operator in the second clause can be either == or ===. The code author worked hard to make sure that the left operand was never undefined in practice; my static analysis shows that it could become undefined ... although I am unable to arrange the world so that it happens. Maybe a failure of imagination.

My static analysis of the minified code says it is correct although my minified version is different from yours, perhaps because mine is minified against an evolving copy of breeze.debug.js (somewhere closer to what v.1.4.9 will be).

// Reformatted w/ spaces and new lines for readability. 
// Observe that the minifier reversed the direction of the second null test!
// That is smart and does no harm
// I can assure you the pre-minified code is the same as what you folks are reporting.
function m(a,b) {
    if(a._context){
          for(var c=a._context; null!=c.prevContext;) c=c.prevContext;
          if(null === c.prevContext) return c.prevContext=b, a;
          if(null !== b.prevContext)
               throw new Error("Illegal construction - use 'or' to combine checks");
          b.prevContext=a._context
    }
    return n(a,b)
}

Under these trying circumstances, unless we can find a failing test, we'll make a leap of faith, slaughter a chicken, rattle some bones, and change the code to this:

if (curContext.prevContext === null) {
    curContext.prevContext = context;
    // just update the prevContext but don't change the curContext.
    return that;
} else if (context.prevContext == null) {  // CHANGED TO "if null or undefined"
    context.prevContext = that._context;
} else {
    throw new Error("Illegal construction - use 'or' to combine checks");
}

If you can make the time, please try this in your apps and confirm that all is well.

We're shipping v.1.4.9 tomorrow (28 Feb) so please try this pronto!

Community
  • 1
  • 1
Ward
  • 17,793
  • 4
  • 37
  • 53
  • Considering that mathias999us reports ** Aaargh! 'curContext.prevContext': undefined 'context.prevContext': null. Is the fix to test one or both values for falsy instead of ===null? Or is the undefined value a problem/unexpected? – steve Feb 25 '14 at 23:32
  • Any luck on your investigation? Looks like other people are having the same issues http://mosaicvivarium.blogspot.no/2014/02/known-issue-error-messages-especially.html – GETah Feb 26 '14 at 13:53
  • I can't reproduce this locally even with the minified version. The issue does however occur when running in production. I think there is a timing factor in the equation... – GETah Feb 26 '14 at 18:12
  • 1
    @Ward Just a heads up: In my case, when I change `curContext.prevContext === null` to use `==` the code throws the error "Error configuring an instance of 'JsonResultsAdapter'. The 'extractResults' parameter must be a 'function'" on the first page load. It's thrown from line 919 in breeze.debug.js version 1.4.8. This happens every time, consistently. – larspars Feb 27 '14 at 13:02
  • I spoke in haste. I'm looking. FWIW, the minification, while weird looking with its re-use of 't' is perfectly valid and does not recurse as you might think. Try the following in Chrome v.33 console; it the inner 't' is the parameter passed in, not the fnc: `(function t(n, t) { console.log(t.prevContext); console.log(t);})('this',{prevContext:'foo'})` – Ward Feb 27 '14 at 22:05
  • @larspars You should only change === to == in the [} else if (context.prevContext === null) ], not in [if (curContext.prevContext === null)] – zorro-lp Feb 28 '14 at 02:30
  • 1
    Just updated to the Breeze 1.4.9, looks like the bug is now gone – GETah Mar 01 '14 at 11:47
  • Yep. New version of breeze has been working well for us. – steve Mar 04 '14 at 17:47
3

This started occurring when Chrome updated to version 33. It did not happen in Chrome 32. I downgraded Breeze from version 1.4.8 to version 1.4.7, and this fixed the problem made the problem happen less often.

(The only breaking change listed in the changelog is that contains queries must be escaped in version 1.4.7. So do a word = word.replace(/'/g, "''"); before doing .where("yourColumn", "contains", word))

Edit:

Nope, changing to 1.4.7 did NOT fix this, it just made the problem occur much less often.

larspars
  • 1,640
  • 1
  • 15
  • 30
  • Thank you larspars. I am using Breeze 1.4.1. I don't know if changing Breeze from 1.4.1 to 1.4.7 works. Let me take a try – zorro-lp Feb 25 '14 at 15:41
  • do you experience this error regularly? It's so strange that this error appears so randomly, irregularly on my site. I tried to reproduce it using a specified scenario but could not. I want to reproduce it before applying the Breeze version change to make sure that error will not happen again. – zorro-lp Feb 25 '14 at 16:52
  • Thanks for the tip to try other versions. So far, we has _not_ gotten the error for the debug/unminimized version of 1.4.8. Could it be that the error does not occur unless minimized? Or is the behavior just different -- harder to reproduce the error? – steve Feb 25 '14 at 23:01
  • @itjunkie We do not experience it regularly, sometimes it happens, sometimes not. We have several breeze queries running concurrently, I figured it depended on the order in which they completed. steve, we also did not get the error when using breeze.debug.js locally. I figured it had to with timing differences between localhost and production, but it could very well be that it never happens in the unminified version. – larspars Feb 26 '14 at 08:05
2

Ok, we just released Breeze 1.4.9 with Mathias999us's suggested fix. Please let us know whether this fixes the issue ... or not. Thanks Mathias ;)

Jay Traband
  • 17,053
  • 1
  • 23
  • 44
1

Looking at the code, Breeze is expecting either 'curContext.prevContext' or 'context.prevContext' to be 'null'. One of them has to be 'null' in this check.

The error is thrown when both curContext.prevContext and context.prevContext already are set to a value.

Chi Row
  • 1,106
  • 7
  • 18
  • Although your answer is correct and answers the question asked, I (and maybe the person who asked the question) want to know is: what conditions lead to the error. IOW, what is the root cause of this error? – steve Feb 24 '14 at 20:59
1

For our application, switching to the non-minified (debug) version of breeze 1.4.8 eliminated the error. I cannot say with confidence that this will fix the problem for everyone since I don't know the root cause of the problem or how minification causes the error. Maybe the error can still happen with the non-minified version, but it doesn't in our application.

FWIW: Based on our experience and what I've read from others, the error is characterized in that it:

  • only occurs in the latest version of Chrome (33) -- windows and mac!
  • does not always happen.
  • seems to have a timing aspect. For us it only happens in first few breeze queries after starting the application -- although not necessarily the first query.
  • occurs for every query after the first query that fails.
steve
  • 1,021
  • 1
  • 14
  • 29