[translation-service] PR to make changes to api mock

Programming JavaScript
1

I came upon a student solution that passed all the tests, and for the premium function had something like:

premium(text, minimumQuality) {    
  return this.api.fetch(text)
    .catch(error => {
      this.request(text).catch(() => {});

      return this.api.fetch(text);
    })
    .then( // ... rest of code

(The original version was more verbose than this, but functionally the same.)
At first glance it looks like a good solution: try fetch, interject with request, do fetch again.

But, the second fetch does not chain to request, and yet this passed all the tests.

The issue is two-fold: The success path passes because request prepares the translation synchronously and delays just the reply, so an immediate fetch that doesn’t wait for a reply can be successful.

For the expected failure path, the errors thrown from fetch satisfy the tests’ toThrow(Error), since any child class of Error is itself an Error.

I’ve made changes (PR here) so that the api “work” is delayed in the callback, and introduced a new error type. I think this way it would work closer to a real life service api and as intended by the exercise assignment.

2

But, the second fetch does not chain to request, and yet this passed all the tests.

Ah. Yes. We don’t want that!

The issue is two-fold: The success path passes because request prepares the translation synchronously and delays just the reply, so an immediate fetch that doesn’t wait for a reply can be successful.

This is not necessarily an issue, but I agree that we can have fail fast solutions if we did delay this. Much more like how it would be in real life with a real API!

For the expected failure path, the errors thrown from fetch satisfy the tests’ toThrow(Error), since any child class of Error is itself an Error.

The expected failure path is still fine. By making the request and fetch “delayed”, technically it would be fine. You could then write this (don’t, it’s not good to mix styles):

.catch(async (error) => {
  await this.request(text).catch(() => {});

  return this.api.fetch(text);
})

Throwing inside the catch is the same as returning a rejected promise :smiley:

.

I will re-open your PR and look at the changes but overal sounds like a very good improvement.