orval icon indicating copy to clipboard operation
orval copied to clipboard

MSW: Allow overriding entire handler function in generated MSW handlers, not just response object

Open severinh opened this issue 1 year ago • 4 comments

This is a feature request, not a bug.

What are the steps to reproduce this issue?

#1186 made it possible to override response objects in MSW handlers. Thank you for that! Unfortunately, we often cannot use them in our project in many tests because we're missing two things:

  1. In unit tests, we often override the default handlers to simulate a HTTP 500 errors to test the request failure path.
  2. We sometimes create handlers where the output depends on the request.

Here's an example of a handler generated by Orval:

export const getProjectQueryMockHandler = (overrideResponse?: Result) => {
  return http.post('*/projects/:projectId', async () => {
    await delay(200);
    return new HttpResponse(JSON.stringify(overrideResponse ? overrideResponse : getProjectQueryMock()),
      {
        status: 200,
        headers: {
          'Content-Type': 'application/json',
        }
      }
    )
  })
}

Simulating HTTP errors in tests

Then, for example, a unit test needs to simulate a HTTP 500 error. It currently cannot use getProjectQueryMockHandler for that since that always uses HTTP 200. So we instead don't use any code generated by Orval, and write something like this. Unfortunately, this is a lot less type-safe, since there's nothing checking that the path projects/:projectId is correct.

  server.use(
    http.post('*/projects/:projectId', async () => new HttpResponse('', { status: 500 }))
  );

What we'd love to see is getProjectQueryMockHandler allowing callers to either override the response, or the entire handler function. Something like:

  server.use(
    getProjectQueryMockHandler(async () => new HttpResponse('', { status: 500 }))
  );

Alternative solution: Generate a function to get the path of the endpoint, such as:

  server.use(
    http.post(getProjectQueryPath(), async () => new HttpResponse('', { status: 500 }))
  );

Changing request based on the response

Similarly, we have tests where the MSW handlers need to return a response that depends on what is in the request. So we currently write something like:

  server.use(
    http.post('*/projects/:projectId', async ({ request }) => {
      const input = (await request.json()) as Input;
      // Return a response based on input.
  })),

Ideally, we could write this as:

  server.use(
    getProjectQueryMockHandler(async ({ request }) => {
      const input = (await request.json()) as Input;
      // Return a response based on input.
  }));

Any other comments?

Allowing overriding of the entire handler function is also what @graphql-codegen/typescript-msw offers, which we use for our GraphQL endpoints.

Still, orval is great! I expect it to be a significant boost in DevEx for our team.

What versions are you using?

Package Version: 6.24.0

severinh avatar Feb 06 '24 06:02 severinh

@severinh

Thank you for made this issue.

Simulating HTTP errors in tests

Regarding alternatives, when extending and defining a mock definition in a test, for example, you can use the following function that automatically generates the key.

https://github.com/anymaniax/orval/blob/master/samples/react-app-with-swr/src/api/endpoints/petstoreFromFileSpecWithTransformer.ts#L35


const key = getListPetsKey()

server.use(
  http.post(key, async () => new HttpResponse('', { status: 500 }))
);

soartec-lab avatar Apr 19 '24 08:04 soartec-lab

Regarding alternatives, when extending and defining a mock definition in a test, for example, you can use the following function that automatically generates the key.

Thanks for your reply!

The get*Key methods could be used, but there a number of downsides to that:

  1. The method actually returns an array, where only the first element is the URL. So you would need to do something like http.post(getListPetsKey(123)[0], async () => new HttpResponse('', { status: 500 }))
  2. You still have to repeat the HTTP verb (e.g. http.post). That's a possible source of errors, since the writer of the test has to manually make sure to pick the correct HTTP verb - even though orval actually knows the right verb.
  3. It forces you to actually pick values for the path parameters, such as getListPetsKey(paramA, paramB). That's inconsistent with the Orval-generated handlers, which just uses a placeholder such as :paramA, :paramB in the URL given to MSW.

Overall, I'm afraid this would not be a good developer experience.

So for now, my team will keep hard-coding URLs in tests. And I would be delighted if in the future, Orval will provide support for more flexible handler overrides.

severinh avatar Apr 19 '24 08:04 severinh

Yes, this is an alternative method, but it is consistent and functional with the URL defined in OpenAPI, so please consider it as one method.

const key, _ = getListPetsKey()

soartec-lab avatar Apr 19 '24 09:04 soartec-lab

If you want to test different response types, you can use the generateEachHttpStatus config option in output.mock:

output: {
  mock: {
    type: "msw",
    generateEachHttpStatus: true
  }
}

With a specification that have two responses of a ListPets endpoint, one 200 and one 500, the following mock is generated:


export const getListPetsMockHandler200 = (
  overrideResponse?:
    | Pets
    | ((info: Parameters<Parameters<typeof http.get>[1]>[0]) => Pets),
) => {
  return http.get('*/v:version/pets', async (info) => {
    await delay(1000);
    return new HttpResponse(
      JSON.stringify(
        overrideResponse !== undefined
          ? typeof overrideResponse === 'function'
            ? overrideResponse(info)
            : overrideResponse
          : getListPetsResponseMock200(),
      ),
      {
        status: 200,
        headers: {
          'Content-Type': 'application/json',
        },
      },
    );
  });
};

export const getListPetsMockHandler500 = (
  overrideResponse?:
    | Error
    | ((info: Parameters<Parameters<typeof http.get>[1]>[0]) => Error),
) => {
  return http.get('*/v:version/pets', async (info) => {
    await delay(1000);
    return new HttpResponse(
      JSON.stringify(
        overrideResponse !== undefined
          ? typeof overrideResponse === 'function'
            ? overrideResponse(info)
            : overrideResponse
          : getListPetsResponseMock500(),
      ),
      {
        status: 500,
        headers: {
          'Content-Type': 'application/json',
        },
      },
    );
  });
};

And you no longer need to overwrite the whole mock handler.

AffeJonsson avatar May 24 '24 10:05 AffeJonsson

Thanks @soartec-lab and @AffeJonsson.

Since I originally filed this ticket, #1375 went in, which made it possible to pass a function into the mock handler to produce the response object. Secondly, we've adopted generateEachHttpStatus, as suggested. Thanks to the combination of these two, we were able to get rid of all hard-coded URLs in our tests.

There was still one issue: Our schemas don't contain 500 as an explicit HTTP status code, so no mock handlers are generated for those, even when using generateEachHttpStatus. As a work-around, we're now using something like server.use(getSomeQueryMockHandler(() => throw Error("Server error"));.

While not 100% the same as returning a HTTPResponse.json({}, {status:500}), from the perspective of the application under test it's the same.

In the interest of avoiding confusion, I'll close this issue.

severinh avatar Jul 02 '24 08:07 severinh