Got: RequestError: Cannot read property 'bind' of undefined

[ganapativs]

Describe the bug

• Node.js version: 12.14.0
• OS & version: MacOS 10.15.2

Actual behavior

Request fails with error RequestError: Cannot read property 'bind' of undefined when array of functions is passed to beforeRequest hook.

This bug was introduced after upgrading from version 10.6.0 to 11.3.0. This feature works well till version 10.7.0.

_Demo:_ https://codesandbox.io/s/got-bug-1130-not-working-hs678

_StackTrace:_

{ RequestError: Cannot read property 'bind' of undefined
    at PromisableRequest._makeRequest (/sandbox/node_modules/got/dist/source/core/index.js:1010:19)
    at handleError (/sandbox/node_modules/@szmarczak/http-timer/dist/source/index.js:30:34)
    at Function.timer [as default] (/sandbox/node_modules/@szmarczak/http-timer/dist/source/index.js:42:5)
    at PromisableRequest._onRequest (/sandbox/node_modules/got/dist/source/core/index.js:813:29)
    at PromisableRequest._makeRequest (/sandbox/node_modules/got/dist/source/core/index.js:991:22)
    at process._tickCallback (internal/process/next_tick.js:68:7)name: 'RequestError', code: undefined, timings: undefined }

Expected behavior

Request should succeed when an array of functions is passed to beforeRequest hook.

This feature should be supported as mentioned in https://github.com/sindresorhus/got#hooksbeforerequest.

_Demo:_ https://codesandbox.io/s/got-bug-1070-working-0ih5w

Code to reproduce

const got = require("got");

const gotx = got.extend({
  hooks: {
    // Could be array of functions
    // https://github.com/sindresorhus/got#hooksbeforerequest
    beforeRequest: [o => o, o => o]
  }
});

(async () => {
  try {
    const r = await gotx.post("https://httpbin.org/anything", {
      json: {
        hello: "world"
      },
      responseType: "json"
    });

    console.log(r.body);
  } catch (err) {
    // Fails with 'RequestError: Cannot read property 'bind' of undefined'.
    console.error(err);
  }
})();

In version 11.3.0, everything works fine when a single function is passed to the beforeRequest hook and breaks when multiple functions are passed.

But, the afterResponse hook supports array of functions in version 10.6.0 and 11.3.0 too.
_Demo:_ https://codesandbox.io/s/got-bug-1130-working-afterresponse-ihuy9

Am I missing something or is this a bug? 🤔

Checklist

• [x] I have read the documentation.
• [x] I have tried my code with the latest version of Node.js and Got.

[Giotino]

beforeRequest hooks should not return the new options, if you want to modify them just do it, on the options param.
Documentation says nothing about the return value of beforeRequest, but, looking at the code, it should return undefined or an http.IncomingMessage

[ganapativs]

@Giotino Thanks for investigating this 🙌

Yes, Modifying option without returning works fine 👍
So, if I understood it correctly, returning options from beforeRequest hook was supported before version 11.0.0 and it has changed after that.

Anyways, this solution solves my problem. Closing the issue.

[Giotino]

So, if I understood it correctly, returning options from beforeRequest hook was supported before version 11.0.0 and it has changed after that.

According to the documentation of got 10.7.0 returning something on beforeRequest hook was already undocumented, the "return a response" feature was added in got 11 but never documented.

In 10.7.0 the return value of that hooks was simply discarded

https://github.com/sindresorhus/got/blob/5c74084fe49fe69e3e12e059d6adf46def2af764/source/request-as-event-emitter.ts#L318-L321

In 11 is used as the response

https://github.com/sindresorhus/got/blob/47239e3f29a6ade38e9353d26eaffa3f79466103/source/core/index.ts#L1361-L1370

[ganapativs]

Thanks for the great explanation 🙌

I probably misunderstood this part of the README.

Hooks allow modifications during the request lifecycle. Hook functions may be async and are run serially.

My understanding was when Function[] is defined, the return value of one function is passed as input for the next function.

Anyways, now it's clear 👍

[szmarczak]

@Giotino @ganapativs It's now documented: 779062a1b99a9a1c13737b9fd613e185d97b158b

[ganapativs]

@szmarczak Awesome 🙌