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

Duplicate model exception cannot be caught #976

Closed
ghost opened this issue Feb 8, 2012 · 5 comments
Closed

Duplicate model exception cannot be caught #976

ghost opened this issue Feb 8, 2012 · 5 comments

Comments

@ghost
Copy link

ghost commented Feb 8, 2012

In addition to being unreadable, the following conditional will halt all script execution in the event that it tests positive and doesn't seem to be caught by a try block.

if (cids[cid = model.cid] || this._byCid[cid] ||
          (((id = model.id) != null) && (ids[id] || this._byId[id]))) {
          throw new Error("Can't add the same model to a collection twice");
        }
try {
        this.dashboardFeed.fetch();
      } 
      catch (e) 
      {
        //Never gets here
        console.error(e);
      }

While it's certainly a problem if this ever hits, I'd really prefer it if it didn't crash my entire application as a result.

@braddunbar
Copy link
Collaborator

Hi @khiltd! The reason your error isn't being caught is because add is not executed during the try block. It's deferred until the request is complete. You have several options for making this work, you can override collection.parse and check your data or you can override collection.add and check your data there.

@ghost
Copy link
Author

ghost commented Feb 8, 2012

Makes perfect sense technically, but I'd argue that that's not a great design choice as it forces such error checking into an extremely unnatural location no one is going to remember two weeks later. Why can't you simply refrain from inserting that bothersome object and console.error it out, or better yet, invoke an error callback?

@braddunbar
Copy link
Collaborator

There is some controversy over the desired behavior in this case. For starters, you can check out #836, #708, & #414.

@Tivoli
Copy link

Tivoli commented Feb 11, 2012

Ran into the same thing when adding an array of models. While I like that collections are now unique it's problematic when adding items without overriding the add method, which is what we had to do before anyway for unique collections.

It'd be nice if it just failed silently if there was a duplicate.

@jashkenas
Copy link
Owner

Looks like we're all set, vis-a-vis this ticket. Duplicate model additions are now a no-op on master.

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

3 participants