fix: allow async overrideResponse to be passed to msw handler (issue #1389)

[severinh]

Status

READY

Description

Fix #1389.

In MSW, a mock handler can access the response resolver info. For example, it can access the request body to change the response based on the request. Access to that request body is only possible through an async function call to json().

http.post("/some/path", async ({ request }) => {
  const requestJson = await request.json();
  // Do something with the request, such as producing a response based on it.
  return ...
});

In Orval, PR #1375 made it possible for the override in a MSW mock handler to be a function, and Orval passes the response resolver info to that function. However, Orval requires a synchronous method that returns a response object. It does not allow async functions.

Before this PR, it was not possible to migrate the above mock handler to Orval. The following code would not type check:

getSomePathMockHandler(async ({ request }) => {  // This does not type-check. Orval does not allow a Promise<SomeResponse> return type.
  const requestJson = await request.json();
  // Do something with the request, such as producing a response based on it.
  return ...
});

This PR changes the code generation such that both Promise<ResponseType> | ResponseType is allowed as a return type for the override. This mirrors what MSW does.

Related PRs

#1375

[soartec-lab]

Hi, @severinh
I would like to understand the background a little more, so please tell me specifically when the problem occurs when it is not a Promise. Is it just that type checking doesn't work?

[severinh]

Hi, @severinh I would like to understand the background a little more, so please tell me specifically when the problem occurs when it is not a Promise. Is it just that type checking doesn't work?

It's not just the type-checking that does not pass. The whole mock handler does not work correctly when passing an async method that returns a Promise. Let's assume we ignore the type checking error. The problem is: When passing an async method, the overrideResponse function returns a Promise of a response, not a response object. Orval then directly passes that Promise into JSON.stringify.

Orval only supports passing a non-async method right now. The following currently works in Orval:

// Suppose the user passes the following overrideResponse function:
const overrideResponse = (info) => {
  // Cannot get the request body from info, because that would require the method to be async
  return {"field": "value"}
}
// Orval then does the following call
JSON.stringify(overrideResponse())
which outputs "{'field': 'value'}"

However, suppose the user passes an async function:

const overrideResponse = async (info) => {
  // Do something async with info, such as reading the request body.
  return {"field": "value"}
}
// Orval (before this PR) then does the following call
JSON.stringify(overrideResponse())
// This outputs "{}", which is wrong, because we're stringifying the Promise itself, not the awaited object.
// This needs `await overrideResponse()` to work

Hope that helps.

[soartec-lab]

@severinh

Thank you for your kindness. I understand that much, but what is the specific background of why I want to use Promise?
In the current state, we are only dealing with simple objects or functions that return objects, but adding more `Promises' here will also increase complexity. I would also like to understand the benefits I can receive from this, so please let me know.

[severinh]

@severinh

Thank you for your kindness. I understand that much, but what is the specific background of why I want to use Promise? In the current state, we are only dealing with simple objects or functions that return objects, but adding more `Promises' here will also increase complexity. I would also like to understand the benefits I can receive from this, so please let me know.

Happy to clarify.

There are two concrete use cases:

1. Returning a response based on the request:

Here's what you can do in MSW:

http.post("/greeting", async ({ request }) => {
  const { firstName, lastName } = await request.json();
  return { greeting: `Hello ${firstName} ${lastName}` }
});

The request body is only available via request.json, which returns a Promise. And hence can only be called in an async method.

2. Making assertions in a mock handler on the request in a test:

Here's what you can do in MSW:

http.post("/greeting", async ({ request }) => {
  const { firstName, lastName } = await request.json();
  expect(firstName).toEqual("Test");
  expect(lastName).toEqual("Name");
  return getGreetingResponseMock();
});

Neither of these is possible if you want to use an Orval mock handler. That means you have to fall back to hard-coding the paths - and not having to copy-paste paths across the code base is a major reason for using Orval in the first place. :)

This is related to issue #1206 that I filed some time ago. Personally, I would have have advised against merging #1375, because it only allows partial flexibility. With the current method signature of Orval's mock handlers (introduced in #1375), you can only override the response body, but you cannot override the entire HttpResponse (for example to simulate a HTTP 500 error, or add some more response headers).

My preferred solution would have been for Orval mock handlers to look like this:

export const getGreetingMockHandler = (override?: GreetingResponse | HttpResponseResolver<{ ... }, GreetingRequest, GreetingResponse>) => {
  return http.post('/greeting', async (info) => {
    return typeof override === 'function'
      ? await override(info)
      : new HttpResponse(JSON.stringify(override !== undefined ? override : getGreetingResponseMock()), {
          status: 200,
          headers: {
            'Content-Type': 'application/json',
          },
        });
  });
};

// One could then use this Orval mock handler like this:
server.use(getGreetingMockHandler(async (info) => {
  // Can produce a response based on the request, make assertions, set custom headers, etc.
  return new HttpResponse(...);
});

This would be both simpler and more flexible than what #1375 introduced. But the problem is that since #1375 was already released in 0.29, this would be a backwards-incompatible change. Would that be a possibility? If yes, I'm happy to prepare a PR to make the mock handlers look like the above, solving #1206 too.

[soartec-lab]

@severinh

First of all, regarding 1, you want to perform calculations in request and calculate the value of the return type object, right? And, on the side that uses mocks, you can do the same thing by creating and using an object that is not a Promise from the beginning, right?

[severinh]

@severinh

First of all, regarding 1, you want to perform calculations in request and calculate the value of the return type object, right? And, on the side that uses mocks, you can do the same thing by creating and using an object that is not a Promise from the beginning, right?

Thanks for the patience. I'm afraid I don't fully understand the question. What do you mean by "on the side that uses mocks"? Could you explain how you would do 1) with an Orval-provided MSW handler like getGreetingMockHandler?

Maybe what could be helpful context: I could of course say: "Fine, for cases like 1) and 2), I just won't use Orval, and instead write the MSW handlers from scratch with http.post('/greeting', ...), hard-coding the HTTP verb post and path. That works.

But as I mentioned in #1206, the main reason why we use Orval is exactly because it lets us avoid having to hard-code HTTP verbs and paths like post and '/greeting' in application and test code, but use type-safe, generated code instead (getGreetingMockHandler). From the perspective of developers in our team, it's confusing if for some types of mocking cases, they can use the Orval-generated handlers, and for others, they have to use plain, unsafe MSW instead. Avoiding that is the main reason why I opened the ticket and PR. :)

[soartec-lab]

OK. In short, it's not clear whether you want to use Promise to optimize it individually for your complex project, or if it will benefit you as a whole.
This change itself is minor, so I would like to merge it if there is any benefit.
However, when using the original handler, if a response object or calculation is required, wouldn't it be sufficient to input the function?
For example, in the case of getGreetingMockHandler, wouldn't it be sufficient to just pass the function without doing any asynchronous processing?

const fullname = (firstName: string, lastName: string) => {
  return { fullname: `${firstName} ${lastName}`
}

getGreetingMockHandler(fullname)

[severinh]

Ah, now I understand the confusion.

In short, it's not clear whether you want to use Promise to optimize it individually for your complex project, or if it will benefit you as a whole.

To clarify, this PR:

• has nothing to do with optimization.
• is not about using async because something is slow or complex.
• is not about building a fixed mock response. You're right that to build a mock response, we can just use getGreetingMockHandler({greeting: "Hello world"}). That works perfectly.

This PR about accessing the HTTP request (!) that the application made to the MSW request handler, which is currently not possible with an Orval-created request handler.

Probably my toy example 1) is too simple, so it's not clear why this would be useful.

Right now, with Orval request handlers you can only use a static mock response object. No matter what request the application sends, the request handler will return the same response object.

In the real server-side implementation of an API, the response will be produced based on the request. Now, it does not make sense to reimplement the real server implementation in a request handler. However, in specific cases, it can help to make the behavior of the request handler just small bit more realistic, by changing the response dynamically (!) based on what request the application made. But that requires access to the request that the application sent, and producing a response object based on that. But Orval currently does not allow that kind of access.

Maybe for 2), I can give you a more realistic example, actually from our codebase. We have a functionality where the user can click a thumbs up/thumbs down button in the app, which sends a request to the server of the form { sentiment: 'positive' } or { sentiment: 'negative' }. We have the following test, which asserts that the application actually makes the right request to the server.

server.use(
  http.post('*/projects/:projectId/feedback', async ({ request }) => {
    const { sentiment } = (await request.json()) as FeedbackRequest;
    expect(sentiment).toBe('positive');  // Asserts that the right request is sent.
    return HttpResponse.json(getRecordFeedbackMutationResponseMock());
  })
);
await userEvent.click(screen.findByRole('button', { name: 'Useful' }));

The above is not possible with Orval. I'd like to be able to write the following test using the Orval-generated getRecordFeedbackMutationResponseMockHandler, but I cannot:

server.use(
  // Orval does not support this async function
  getRecordFeedbackMutationResponseMockHandler(async ({ request }) => {
    const { sentiment } = (await request.json()) as FeedbackRequest;
    expect(sentiment).toBe('positive');  // Asserts that the right request is sent.
    return getRecordFeedbackMutationResponseMock();
  })
);
await userEvent.click(screen.findByRole('button', { name: 'Useful' }));

How about the following: I'm fine with leaving this PR open for now, since it only addresses half of my problem. It only partially resolves #1206. If it's okay for you, I can try to write a new PR to solve #1206. And the solution to that would actually lead to cleaner generated code. I'm about to go on vacation though, so will not respond for a week. :)

[soartec-lab]

@severinh

Hmm, I understand the scene you want to use. You want to reproduce the case where the http response changes dynamically depending on the http request, so you want to execute that logic, right?

Also, for example, if you want to test whether the request body of an http request is correct, request.json() can only be handled by Promise.

well understood. I agree with this so I'll merge it. Thank you for teaching me so carefully.

Let's discuss #1206 in an issue.

[soartec-lab]

thanks!