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

Implement a custom Error #405

Closed
wants to merge 2 commits into from

Conversation

jugglinmike
Copy link
Collaborator

This occurred to me when reviewing #404. It would be easy to simply update the test to ensure that some Error was thrown, but I think we can do better. Not only does this change make writing this test easier, it also makes using LayoutManager easier:

Currently, if a developer has some special logic to recover from (or log) LayoutManager issues, there's no obvious way to do this:

function safeInsert(layout, view) {
  try {
    layout.insertView(view);
  } catch (err) {
    if (/* how do I write this safely? */) {
      // Recover from (or log) the LayoutManager error
    } else {
      throw err;
    }
  }
}

With a custom Error class, this is simple:

   } catch (err) {
-    if (/* how do I write this safely? */) {
+    if (err instanceof Backbone.Layout.Error) {
       // Recover from the LayoutManager error

I'll admit that it does seem a little unlikely that people will need this for LayoutManager's one and only Error. This also isn't a pattern I've seen in other JavaScript libraries. That said, having it around (and documented) means that we can easily reach for it in the future (where Errors thrown from more dynamic code paths would make it more important).

Errors thrown by LayoutManager should be considered part of the API. To
that end, developers should expect a consistent interface that they can
write code against. By subclassing Error, LayoutManager can generate
errors which can be handled (or rethrown) by application code in an
intuitive manner.
The intent of this test is to assert that an error was thrown by
LayoutManager. The error message itself (while important) is not a
relevant concern for a unit test--asserting it here only makes the
library more difficult to maintain.

Additionally, move the assertion outside of the control flow statement
to avoid potential false positives.
@SBoudrias
Copy link
Collaborator

I don't know how useful this can be in LM.

But more generally, I find this to be a quite clever way of allowing error recovery. I'll sure keep it in mind.

@jugglinmike
Copy link
Collaborator Author

Although I like this change, it seems like nobody likes it so I'm going to close for now

@jugglinmike jugglinmike closed this Apr 4, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants