Got: `request._header` is not present

[gajus]

Describe the bug

• Got version: ^10.0.0-beta.1
• Node.js version: v13
• OS & version: macos

Actual behavior

Making HTTP POST request to a resource that returns response:

HTTP/1.1 302 Found
Server: Server
Date: Mon, 18 Nov 2019 18:51:50 GMT
Content-Type: text/html;charset=UTF-8
Content-Length: 0
Connection: close
X-XSS-Protection: 1
X-Content-Type-Options: nosniff
X-Frame-Options: SAMEORIGIN
x-ua-compatible: IE=edge
Pragma: No-cache
Cache-Control: max-age=0, no-cache, no-store, must-revalidate
Expires: Thu, 01 Jan 1970 00:00:00 GMT
Location: https://[..]
Set-Cookie: [..]
x-amz-rid: 7FYHFS7W55AF5YAGF84X
Vary: Accept-Encoding,X-Amzn-CDN-Cache,X-Amzn-AX-Treatment,User-Agent

Produced an error (?):

`request._header` is not present. Please report this as a bug:
https://github.com/sindresorhus/got/issues/new?title=%60request._header%60+is+not+present&body=It+causes+%60uploadProgress%60+to+fail.

After that Got just hangs.

Expected behaviour

Follow Location redirect/ return response.

[gajus]

Looks like it is coming from here:

https://github.com/sindresorhus/got/blob/518f0f5906e1fcd38691555b17fb91db643f3d47/source/progress.ts#L77-L86

Looks like whatever it was, doesn't exist in v13.0.1.

[gajus]

Not clear why would the uploadProgress be needed here at all. I am doing FormData POST.

const formData = new FormData();

formData.append('metadata1', 'true');

const response = await got('https://[..]/signin', {
  body: formData,
  cookieJar,
  headers: {
    'Accept-Language': 'en',
  },
  method: 'post',
  throwHttpErrors: false,
});

[szmarczak]

A RunKit example would be lovely.

[szmarczak]

This is actually really weird because I've studied Node.js source code very closely and couldn't find a single way where _header wouldn't be a string.

[szmarczak]

Follow Location redirect/ return response.

It does that, it just emits a warning.

[gajus]

It does that, it just emits a warning.

It didn't. It was just hanging.

I am wondering if this is related to https://github.com/sindresorhus/got/issues/876.

I will try to create a reproducible case.

[szmarczak]

I did forget something.

https://github.com/sindresorhus/got/blob/52b7fc58ea0007e55d95f38fb480d7a951a4895e/source/request-as-event-emitter.ts#L203-L219

As you can see, the only case where request.end() is not called is when the body is a stream.

So basically I need to revert fix:

https://github.com/sindresorhus/got/pull/921/files#diff-7b439d0c6f823422fb34bf344e2e149cL69

because ._header is created on the very first write:

https://github.com/nodejs/node/blob/0f58bfd0633e357208822a1bc045c5e3bbbb3f6e/lib/_http_outgoing.js#L623-L639

/cc @sindresorhus

[szmarczak]

Basically it is needed to overwrite request.write.

[hyperniklas]

To fix your problem you can insert - https://github.com/sindresorhus/got/blob/52b7fc58ea0007e55d95f38fb480d7a951a4895e/source/progress.ts#L74:

if (request._header === null) {
  return;
}

const lastUploadedBytes = uploadedBytes;

However, did you just stumble upon an old bug? (I have only analyzed this isolated case, so input values might not be possible)
1) this._header - https://github.com/nodejs/node/blob/db706da235d80e5dadaab64328bfad9cb313be39/lib/_http_outgoing.js#L107 = null;

2) if (typeof data === 'string' && (encoding === 'utf8' || encoding === 'latin1' || !encoding)) - https://github.com/nodejs/node/blob/db706da235d80e5dadaab64328bfad9cb313be39/lib/_http_outgoing.js#L257
can enter with case (can this occur at first call?):
data = null;
encoding = (encoding === 'utf8' || encoding === 'latin1' || !encoding)
fix?
if (data && typeof data === 'string' && (encoding === 'utf8' || encoding === 'latin1' || !encoding)) {

Expression test: (data = this._header + data;)
expression:

null + null = 0

current value:

data = 0;

Later: - https://github.com/nodejs/node/blob/db706da235d80e5dadaab64328bfad9cb313be39/lib/_http_outgoing.js#L280
_return this._writeRaw(data, encoding, callback);_
expression:
this._writeRaw(0, encoding, callback);
Later: - https://github.com/nodejs/node/blob/db706da235d80e5dadaab64328bfad9cb313be39/lib/_http_outgoing.js#L304

// Directly write to socket.
return conn.write(0, encoding, callback);

Later: -https://github.com/nodejs/node/blob/db706da235d80e5dadaab64328bfad9cb313be39/lib/_http_outgoing.js#L307

// Buffer, as long as we're not destroyed.
this.outputData.push({ data, encoding, callback });
// example first value [ { data: 0, encoding: 'latin1', callback: null } ]

_A) Next write if: this._header === null_
[ { data: null, encoding: 'latin1', callback: null } ]
and later:
outputData.unshift({ data: null, encoding: 'latin1', callback: null });
current outputData:

[
  { data: null, encoding: 'latin1', callback: null },
  { data: 0, encoding: 'latin1', callback: null }
]

_B) Next write if: this._header !== null_
[ { data: 'test', encoding: 'latin1', callback: null } ]
and later:
outputData.unshift({ data: 'test', encoding: 'latin1', callback: null });
current outputData:

[
  { data: 'test', encoding: 'latin1', callback: null },
  { data: 0, encoding: 'latin1', callback: null }
]

BUT BEFORE that would happen(i.e. after unshift call):

this.outputSize += header.length; // header = null: null.length => TypeError: Cannot read property 'length' of null
this.outputSize += header.length; // header = 0
this.outputSize === undefined // true

Other possible problems:
proxy-agent - https://github.com/nodejs/node/blob/9fbad51c8a6afdaf7d24d57a1712319346efe28e/deps/npm/node_modules/http-proxy-agent/index.js#L93-L99

[szmarczak]

Sorry, but this is not gonna solve the problem because:

1. sockets can be reused
2. HTTP2 sockets can be used simultaneously

[szmarczak]

and you cannot write directly to a socket

[hyperniklas]

1 and 'and you cannot write directly to a socket') I am just citing nodejs source where it explicitly says // Directly write to socket., and at another link to nodejs source // This is a shameful hack to get the headers and first body chunk onto the same packet., did you not bother to look at the code/links?
request._header is only null before the first write (or when reset as in the proxy example), so a null check should be made before/during first call before the interval in order to not call this code every time after the first call? (using a hook or similar)
2) ok, but I am basically saying the bug is in nodejs source code and not in your code.

[szmarczak]

and at another link to nodejs source

You haven't included any links here.

a null check should be made before/during first call before the interval in order to not call this code every time after the first call? (using a hook or similar)

I'm trying to fix this using a Transform stream. I'll let you know.

I am basically saying the bug is in nodejs source code and not in your code.

This is not a bug in Node. It's just a missing feature that should've been existing from the start.

[hyperniklas]

Maybe you didn't click the links so I made them verbose now, just for you.

[szmarczak]

Anyway this is fixed now, appreciate the help! :)

[szmarczak]

Maybe you didn't click the links so I made them verbose now, just for you.

Weird. I was on my mobile and didn't see these. I saw your comment history and you're right - there were links.

[szmarczak]

Thank you guys for a blazing fast reply :zap: