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

• I have read the documentation.
• 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

got/source/request-as-event-emitter.ts

Lines 318 to 321 in 5c74084

	for (const hook of options.hooks.beforeRequest) {
	// eslint-disable-next-line no-await-in-loop
	await hook(options);
	}

In 11 is used as the response

got/source/core/index.ts

Lines 1361 to 1370 in 47239e3

	for (const hook of options.hooks.beforeRequest) {
	// eslint-disable-next-line no-await-in-loop
	const result = await hook(options);
	if (!is.undefined(result)) {
	// @ts-ignore Skip the type mismatch to support abstract responses
	options.request = () => result;
	break;
	}
	}

[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: 779062a

[ganapativs]

@szmarczak Awesome 🙌