Allow beforeRetry hooks to replace the request or response

[ux-engineer]

I'm needing to add search parameter to the URL of the Request object in successive retry requests. That parameter is returning in the body of the first request's response.

As Request is immutable object, I'm rebuilding it as in this SO answer: https://stackoverflow.com/a/54502243/2442714

With hook like:

({  request  }) => {
  if (retryCount > 0) {
    const id = 2222;
    const newRequest= appendQueryToReq(request, `processId=${id}`);
    return newRequest;
  }
  return request; // Actually could omit this return
},

However, I'm finding that even when I'm returning the new request object from beforeRetry hook, it doesn't affect the request that gets run.

Which is apparent when looking at the current source code. const hookResult gets used only for checking if it equals stop: https://github.com/sindresorhus/ky/blob/9876da129c216b99ad9f5760d38ba0ed1876d20e/index.js#L418-L433

[ux-engineer]

Ah, actually I misread type signature for this hook:

https://github.com/sindresorhus/ky/blob/764bc1cdabdb65bbcd9830b0f8bfbd95fed3179a/index.d.ts#L16-L22

However, this really limits what can be done in beforeRetryHook... Basically I can't even modify headers.

So documentation doesn't reflect what is actually going on?

This hook enables you to modify the request right before retry. Ky will make no further changes to the request after this. The hook function receives an object with the normalized request and options, an error instance, and the retry count. You could, for example, modify request.headers here.

[sholladay]

Use request.headers.set() to change the headers. Unfortunately, there's no way to change request.url, even when cloning the request. IMO that is a very important feature and the lack of it is an issue with the spec.

That said, your idea of returning a new request from beforeRetry sounds good to me.

[ux-engineer]

@sholladay thanks for your input. Indeed, this turned out to be feature request.

Any chances of getting it implemented soon?

In the mean time we just might implement the query parameter as a header in this case. But this feature should definitely be there and the current documentation is misleading in this regards.

[ux-engineer]

@sholladay are you sure setting headers even works, given how hookResult is used in code only for checking if it is stop

[sholladay]

Yes, I'm sure our documentation is correct. We have tests that prove it.

https://github.com/sindresorhus/ky/blob/764bc1cdabdb65bbcd9830b0f8bfbd95fed3179a/test/hooks.js#L348-L379

The reason this works is because you are modifying the request object that Ky already has, the return value is thus not needed.

[ux-engineer]

Ah, it's passed by reference then.

This part of the documentation is misleading still: "This hook enables you to modify the request right before retry."

By nature Request contains Headers object, it has methods for modification. Other than that, Request implements Body, however this seems to imply body's content may not be modifiable?

Note: The Body functions can be run only once; subsequent calls will resolve with empty strings/ArrayBuffers.

That would mean that currently only Request headers can be modified.

[sholladay]

Ah, it's passed by reference then.

Yes. All objects in JavaScript are passed by reference. With some APIs, plain objects might get shallow copied first before being passed, but we can't do that with request objects even if we wanted to. And request.clone() has some issues, too.

That would mean that currently only Request headers can be modified.

Sure, but that's not a Ky limitation. It's just how the Request class works. If you think you can make the docs clearer without making them overly complicated, that would be very welcome.

And again, being able to return a request is a great idea, especially because the request object is mostly read only.

[sholladay]

We should also allow these hooks to return a response. That will make the API of all hook types more uniform and will enable cleaner syntax for the use case described in https://github.com/sindresorhus/ky/discussions/352.

[kazzkiq]

Are there any plans on implementing this feat? Just stumbled upon this very issue where I'm trying to return a new request with modified request.url but ky simply ignores it.

[vampolo]

Having the same issue as well. this is particularly important, especially with HMAC validation where the URL has to be modified to have a token.

[kdanet]

It's really actually for me! I wanna replace request.url in beforeRequest hook, but I can't do it!