Class invariants and rescue clauses

[estaylorco]

First, thanks so much for a fantastic library!

I was an Eiffel programmer once upon a time and miss DbC. Babel Contracts is the closest I've seen to Eiffel's implementation, and the smoothest. I have already started adding contracts to my code.

Two additions to the library would be most welcome:

• Class invariants
• rescue (as Eiffel would call it)

For the former, imagine this: We place an invariant structure on the constructor of a class, and that invariant is then automatically applied to the beginning and end of each method call (although I admit that I'm not sure what to do about getters and setters). The invariant would not be applied to the constructor, of course, as the instance is still developing.

As for the latter, a rescue clause would allow us to do the following:

• Restore the method to a stable state
• Intercept the exception Contracts throws when the contract is violated, and then re-throw it, most likely with a new Error (and the violation as an inner error)
• Introduce a violation() method for capturing the contract exception thrown by Contracts

Consider the following trivial example:

setStatus(status) {
   require: {
      status;
      typeof status === 'string';
   }

   this.status = status;

   ensure: {
      this.status === 'status';
   }

   rescue: {
      throw new Error('Status was not set', violation());
   }
}

Do you think these things are doable?

Thank you!

[phpnode]

Glad to hear you like the library!

For class invariants, it's doable but there are some limitations, e.g. the invariant won't be picked up by methods in subclasses, and I'm a bit unsure how the syntax should look. Eiffel uses the invariant keyword at the class level, but we can't do that (it'd be a syntax error), so we'd have to use a different keyword and it'd have to be specified in the constructor, e.g.

class Demo {
  constructor() {
    this.neverChanges = true;
    always: {
      this.neverChanges === true;
    }
  }

  test() {
    return this.neverChanges;
   }
}

could compile to

class Demo {
  constructor() {
    this.neverChanges = true;
  }

  test() {
    if (!(this.neverChanges === true)) { throw /* ... */; }
    const result = this.neverChanges;
    if (!(this.neverChanges === true)) { throw /* ... */; }
    return result;
   }
}

I'm a bit unfamiliar with rescue, I guess the easiest way to deal with it is by wrapping any contract code in a try catch block, e.g.

function demo(a) {
  post: a > 123;
  rescue: {
    this.b = old(this.b);
    throw violation();
  }

  this.b = a;
}

could compile to:

function demo(a) {
  const oldThisB = this.b;
  this.b = a;
  try {
    if (!(a > 123)) {
      throw /* ... */;
    }
  }
  catch (e) {
     this.b = oldThisB;
     throw e;
   }
}

[estaylorco]

Wow! Thanks for getting back so quickly.

Yes, I thought about classInvariantas the name. But since you allow for redefining the language, it would only be a matter of coming up with a suitable default name. Perhaps calling it what it is would be the best initial approach: classInvariant.

But then always works as well:

always: {
   accountBalance >= 0;
}

which, I confess, I would redefine to classInvariant.

Rescue clauses are an essential part of DbC in Eiffel. We need to do something when a contract is violated before we return to the client. I like your approach, but I thought I would give you the full purview of rescue in Eiffel.

Below is a sample from actual code in a project I'm working on:

  /**
   * Set the default request timeout (in milliseconds).
   * @type {number}
   */
  set defaultRequestTimeout(interval) {
    require: {
      interval, 'Request timeout interval required';
      typeof interval === 'number', 'Request timeout interval must be a number';
    }

    if (interval < 0) {
      interval = 1000;
      log.warn('Negative request timeout interval coerced to 1 second');
    }

    this._defaultRequestTimeout = interval;

    ensure: {
      this._defaultRequestTimeout === interval, 'Default request timeout was not set';
    }
  }

With a rescue clause--which I would place below ensure for conceptual reasons--this simplifies to this:

set defaultRequestTimeout(interval) {
    require: {
      interval, 'Request timeout interval';
      typeof interval === 'number', 'Request timeout interval must be a number';
      interval >= 0, 'Request timeout must be positive or zero';
    }

    this._defaultRequestTimeout = interval;

    ensure: {
      this._defaultRequestTimeout === interval, 'Default request timeout was set';
    }

    rescue: {
       retry(1000), 'Request timeout interval coerced to 1 second';
    }
  }

I neglected to mention that retry() is a part of the rescue semantics [in Eiffel]. Rather than side-effect manipulation of the result, we retry with a known valid, and run back through the contract.

Another scenario for rescue is this:

rescue: {
    attempts = attempts + 1;
    if (attempts > MAX_AJAX_RETRIES) {
       retry();
    }
}

where somewhere in the body of the method an AJAX call is being made (and failing an assertion). However, a retry is not necessary. In fact, often times rescue clauses are simply used to clean up network resources (close sockets, network connections, disconnect from remote services, etc.)--much like a finally clause but driven by contractual obligations, not side effects.

[estaylorco]

One additional point. You pointed out that subclasses wouldn't pick up the "class" invariant. To tell you the truth, I didn't even explore subclassing, for two reasons:

• Inheritance is fraught with all sorts of problems in JavaScript, as you know
• Contracts become quite complex in the presence of inheritance (you only need to read the relevant section in Eiffel: The Language, 3rd Edition to glean that)

For the back end, I use Spring, which has dependency injection baked right into the framework. I'm using Aurelia for the front end, which, surprisingly, provides almost all of the DI features of Spring with its own DI. So, I'm using DI exclusively these days (no inheritance).

[phpnode]

retry() will run the entire function again, with the provided arguments, is that correct? To make sure I understand the semantics correctly, please would you mind rewriting the defaultRequestTimeout setter above in plain JS (no contracts) so that it behaves as you'd like rescue and retry() to, it'd be a big help, thanks!

[estaylorco]

Rewrite without contracts? Well, now you're asking a lot :)

But, yes, retry() will run the function again, with the provided arguments. As for the rewrite:

this.messenger = new Messenger();
try {
   this.messenger.defaultRequestTimeout = -5;
} catch(e) {
   if (e instanceof SignError) {
      this.messenger.defaultRequestTimeout = 1000;
   }
}

export class Messenger {
   constructor() {
      this._defaultRequestTimeout = 0;
   }
   set defaultRequestTimeout(interval) {
      if (interval < 0) {
         throw new SignError('Negative interval not allowed'); 
       }

       this._defaultRequestTimeout = interval;
   }   
}

[phpnode]

I'm trying to figure out what code with rescue should compile to, so far I'm guessing something like this:

set defaultRequestTimeout(...args) {
  _retrier: while (true) {
    let interval = args[0]; // parameters are moved here so they can be overwritten
    try {
      if (!(interval)) { throw new Error('Request timeout interval');
      if (!(typeof interval === 'number')) { throw new Error('Request timeout interval must be a number');
      if (!(interval >= 0)) { throw new Error('Request timeout must be positive or zero');
    }
    catch (e) {
      args = [1000];
      continue _retrier; // try again
    }
    this._defaultRequestTimeout = interval;

    try {
      if (!(this._defaultRequestTimeout === interval)) {
        throw new Error('Default request timeout was set');
      }
    }
    catch (e) {
      args = [1000];
      continue _retrier;
    }
    break; // out of the loop and the function ends.
  };
}

This is verbose but simple enough, how should the following work though?

rescue: {
    attempts = attempts + 1;
    if (attempts > MAX_AJAX_RETRIES) {
       retry();
    }
}

What is the scope of attempts here? is it just a normal variable defined somewhere in a higher scope? or should it be private to the method invocation, with the value persisting through retries?

[estaylorco]

Even in vanilla, it's still very readable, clear. Very impressive. Did you take into consideration assertions? Mid-method assertion failures might trigger a rescue as well.

You know, I was a bit careless with the second example. It should be this:

rescue: {
    attempts = attempts + 1;
    if (attempts <= MAX_AJAX_RETRIES) {
       retry();
    }
}

And, yes, attempts is a method-level (private) variable that must pass through to the next retry. A design principle of retry is that it cannot mutate the state of the instance.

The next question, you might ask, is what happens if there is no retry in the rescue, or there is, but it's not on the logical path. In Eiffel, a method that throws an exception but never encounters a retry, simply fails. What that means is that the exception is simply passed to the caller until it's either handled, or it encounters another retry. An exception that is never handled and never encounters a retry is simply an unhandled exception.

This is great it terms of the design of your plugin: If rescue and rescue-retry are never used by the developer, the normal exception semantics of JavaScript will prevail. The same is true for rescue-only scenarios: the rescue clause is exception, but an Error is thrown afterwards in the usually way.

Does that make sense?

[phpnode]

Yes that makes sense, thanks. I guess the other question is should rescue only catch errors in contracts, or should it catch any exception? I gather that Eiffel doesn't have try / catch, so I guess it doesn't make a distinction, but it seems like catching everything would be more generally useful, it would allow retrying functions that don't even have contracts. I also wonder if it's worth having a default retry limit of 1 rather than inevitable infinite loops.

There are some complexities around sharing values between retries, I don't really want to have to add yet another label to mark a variable as 'local', and whatever solution has to play nicely with static analysis tools like Flow and other developer tooling (e.g. we can't just come up with a special syntax for declaring a totally new variable), this bit needs some more thought I think.

[estaylorco]

So, before I answer those questions, I should say that I'm going to defer to your sensibilities with respect to JavaScript in general. I say that because neither my goal, nor I'm sure yours, is to try to re-create Eiffel. Eiffel, for obvious reasons for me and I think in general, is probably the go-to language for considering DbC.

I'm not sure, then, the implications of not allowing local variables to pass through from retry to retry. It might just be that that's a limitation in what can be accomplished (but a minor one, certainly). I wonder though if we're simply talking about a memoization pattern, in a sense, which we do quite frequently in JavaScript. Just thinking out loud.

With that said...

• Rescue in Eiffel handles both contracts and developer-raised exceptions (as Eiffel calls them). I agree with you that it would be generally more useful, and would welcome this strategy. I was hesitant to recommend this initially because I thought it might lead to a new exception semantics. It turns out that that's not so.
• Eiffel will allow the developer to walk into an infinite retry. I would preserve this. I think that limiting the retry might be too opinionated (although I definitely share your thought on this).
• Eiffel allows for advanced exception handling in rescue clauses (again, as Eiffel calls it)

The latter is the most interesting, and I apologize for not having brought it to your attention. I actually thought there would be no need to address it. But consider the following precondition:

require: {
   interval > 0, 'Interval must be positive';
   typeof message === 'string', 'Message to display must be of type string';
}

This would indicate that we could violate the contract in two ways. I'm going to suggest a slight enhancement to your library that could stand apart from our discussion, but stay tight to the current JavaScript spec. Consider the following:

require: {
   interval > 0, SignError; // <- custom error class
   typeof message === 'string', MessageError; // <- custom error class
}

rescue: {
   if (error() instanceof SignError) { // error() would be a method introduced into Babel Contracts
      retry(1000); 
   } else if (error() instanceof MessageError) { 
      retry(''); 
   }
   // otherwise, throw a standard JavaScript Error here
}

This is actually exactly how Eiffel works, and it happens to be what we do in JavaScript in general. We're discouraged from creating lots of Error classes. I think that discouragement holds: I'm not proposing that we, as developers, should be creating unique error classes per contract violation. I'm simply saying that, in those cases where we need to make a fine distinction in the rescue clause, we should be able to.

This dovetails nicely with your inclination to raise rescue to a general-purpose mechanism. I think developers might get upset if they have to give up custom errors to embrace rescue, or Babel Contracts in general.

But I mean look at the expressiveness of my second example! So I would say, then, that the following should be permitted in each contract sequence (the first two you already support):

• expression
• expression, string
• expression, Error class

Also, error() should be available only in rescue clauses.

[phpnode]

I agree about not wanting to recreate Eiffel in JS, it's a good source of inspiration though :)

It would be nice to be able to specify an error class, but how then do you specify arguments to the constructor? e.g. I want to throw a TypeError but I also want a custom message.

For preserving variables across retries - if we mutate a local variable in rescue, we can treat it differently, and only assign its value the first time, e.g.

set defaultRequestTimeout(interval) {
  require: {
    interval, 'Request timeout interval';
    typeof interval === 'number', 'Request timeout interval must be a number';
    interval >= 0, 'Request timeout must be positive or zero';
  }
  let totalRetries = 0;
  let someOtherVariable = false;
  this._defaultRequestTimeout = interval;

  ensure: {
    this._defaultRequestTimeout === interval, 'Default request timeout was set';
  }

  rescue: {
    totalRetries++;
    if (totalRetries < 3) {
      retry(1000);
    }
  }
}

Would become:

set defaultRequestTimeout(...args) {
  let totalRetries;
  let _count = 0;
  _retrier: while (++_count) try {
    let interval = args[0]; // parameters are moved here so they can be overwritten
    
    if (!(interval)) { throw new Error('Request timeout interval');
    if (!(typeof interval === 'number')) { throw new Error('Request timeout interval must be a number');
    if (!(interval >= 0)) { throw new Error('Request timeout must be positive or zero');

    if (_count === 1) totalRetries = 0; // Only assigned the first time.
    let someOtherVariable = false; // Not hoisted, because we don't modify it in `rescue`.
    this._defaultRequestTimeout = interval;
    if (!(this._defaultRequestTimeout === interval)) {
      throw new Error('Default request timeout was set');
    }
    break; // out of the loop and the function ends.
  } catch (e) {
    totalRetries++;
    if (totalRetries < 3) {
      args = [1000];
      continue _retrier;
    }
    throw e;
  }
}

There is a nasty bug here though - if the precondition fails, totalRetries will be undefined, so it will fail to increment and retry won't work. In this particular case we can just move the first assignment to the top because it's just a number. The difficulty is when the value of totalRetries comes from somewhere else, like from the result of a method call that might not have happened until after the precondition runs. There'll have to be some restrictions here I think.

btw, I always found it a bit laborious to write contracts for actual type checking, so I usually use this along with flow-runtime and previously babel-plugin-typecheck which let me use Flow's syntax for the types and contracts for everything else, the best of both worlds imho:

function charCodes(input: string): number[] {
  require: {
    input.length > 0;
  }

  const chars = new Array(input.length);
  for (let i = 0; i < input.length; i++) {
    chars[i] = input.charCodeAt(i);
  }
  return chars;
  
  ensure: {
    it.length > 0;
    it.length === input.length;
  }
}

[estaylorco]

It's the developer's responsibility to design a well-formed rescue clause, even in Eiffel. In a totalRetries scenario, we'll call it for short, the developer might need to implement a loop in the method body and place the retry logic there, rather than rely on a rescue-retry. This emerges in Eiffel as well, and it's perfectly acceptable. The developer would be making the following statement:

Retrying this AJAX call up to a maximum number of times is not a matter of contractual stability, but rather a matter of functionality. The method will retry a certain number of times based on configuration (perhaps even runtime configuration), but not based on a contractual obligation to do so.

What I would push to the rescue clause in this case is simply cleaning up network resources, and then eliminate the retry. But what about this restriction:

If a precondition fails, the rescue clause will not execute, and the error specified in the precondition block will simply be thrown.

I say this because I've never seen in Eiffel a stateful rescue clause that doesn't depend on execution of the method body, which means that the preconditions were satisfied. Indeed, I would have to question what it would even mean to have a stateful rescue clause executing if we can't even get to the method body.

As to custom errors, it would be reasonable, I think, to impose a no-args constructor. The error message would be internalized on the instance variable, name, according to the spec. The other minimalist instance variables could be internalized as well, as the StackOverflow discussion goes. Even additional variables could be internalized, such as line number, etc.

But I don't think that limitation would be an issue because, outside the bounds of contracts, the developer could throw an Error or a CustomError in the usual way, with all of the arguments available by way of the language spec. It's only in the context of contracts that we impose the limitation of no-args. This resonates with Eiffel as well. The rescue clause is switching only on species, not species + instance variables. So whether rescue is being used for contracts or for general purposes, we're covered.

Consider this:

require: {
   interval > 0, SignError; // <- custom error class
   typeof message === 'string', MessageError;
   message.length <= 2000, MessageError;
}

rescue: {
   if (error() instanceof SignError) { // error() would be a method introduced into Babel Contracts
      retry(1000); 
   } else if (error() instanceof MessageError) { 
         if(typeof message === 'string') {
            retry(message.substring(1999));  // MessageError must have had to do with length, not type
         }
   }
   // otherwise, pass out whatever uncaught error was thrown

I don't see a loss of expression, really. I could also use unparameterized MessageError in the contract, and parameterized MessageError in the body, and leave off specific trapping of MessageError in the rescue clause altogether. Contracts would simply pass out whatever _Error instance is thrown.

Although Eiffel doesn't have to wrestle with type, the principle is the same: Eiffel anticipates that rescue clauses could be participating in complex exception handling. The rescue clause above is still pretty tame.

This is slightly different from what I said in my code comments about "...throw a standard JavaScript Error here". As a design principle, I would say that if an error of any type is thrown in the method body, and it is not caught by the rescue clause, it should simply pass out as is to the caller.

You know, you would think I would be using Flow or TypeScript considering my background. But as Eric Elliot pointed out, static typing doesn't get you a "correct" piece of software. With contracts, I can check for type and simultaneously make progress towards expressing correctness all in one go.

But, yes, it is laborious. I even question whether or not I should be checking type because, you're right, there are better tools.

[phpnode]

Ah, if preconditions never invoke the rescue clause then I think it's a little easier. I'm not sure when I'll have time to implement these changes, I'm a little busy with work and life at the moment, but I'm definitely keen.

Regarding Flow - it's all about defence in depth, Flow checks types statically, flow-runtime checks types at runtime, babel-plugin-contracts checks everything else at runtime and your test suites cover every other scenario you can think of. Individually they're useful, but put them together and you can write some very robust software :)

[estaylorco]

Hey, no problem as far as time goes. I think we laid a pretty good foundation. You know, it's one thing to use DbC, it's quite another to have a discussion about its design and implementation. I imagine Bertrand Meyer and his team dealt with these same issues decades ago :) In any case, very rewarding for me.

As for me and my part, I think I would like to collect up our discussion into specs, and then post them here, as much for myself as for other readers of these posts. Also, I see three features that are orthogonal to each other. Unless you object, I though I would tease them out into separate feature requests. Might make it easier to track and manage.

As for Flow (or TypeScript), it's coming for me. And certainly I can't argue with the rationale.

Thank you for all of your efforts and, again, for just an awesome plugin!

[estaylorco]

Hey Charles, I just figured out what might be a cleaner way to deal with parameterized Errors and CustomErrors, all within the confines of the JavaScript spec, and as robustly as in Eiffel!

It turns out that LabelledStatements can be nested, according to the spec. Consider this:

require: {
   nonnegativeInterval: interval > 0, SignError;
   intervalMustBeNumberType: typeof interval === 'number';
   validMessageLength: message.length < 2000, MessageError;
}

This approach allows the developer to avoid magic strings, but use them if he wants for standard Error, as in the following hyped-up example:

intervalMustBeNumberType: typeof interval === 'number', 'Interval must be of type number';

-OR-

typeof interval === 'number', 'Interval must be of type number'; // Opt out of labelled clauses

Your plugin would then grab the label, and convert it according to this convention:

intervalMustBeNumberType becomes 'Contract violation: Interval must be number type'

...then inject it into the Error or CustomError constructor as an argument. The Error type in the latter clause above would be instantiated this way:

new MessageError('Contract violation: Valid message length')

Eiffel leverages the labels on contract clauses this way. Also, this would probably play more nicely with autocompletion/intellisense.

I could even see someone else doing another plugin that provides autocompletion around contract clauses in a more formal way. Type inte..., and intervalMustBeNumberType: typeof interval === 'number' shows up in the list.

Just a thought...