Calling forEach on HttpHeaders crashes

[alextekartik]

Thanks a lot for your various interop libs that I'm trying to use to write firebase functions

I tried with the following test and in a firebase functions to call forEach

test('forEach', () {
  Map map = {};
  headers.forEach((key, value) {
    map[key] = value;
  });
  expect(map['content-type'], [headers.contentType.toString()]);
});

This was giving the following error:

TypeError: Cannot read property 'getHeaderNames$0' of undefined

I created a fix in a fork: https://github.com/tekartik/node-interop/commit/f604820ea4af43371d610e1f7b04485f1be6df16

Basically I simply replaced

var names = nativeResponse.getHeaderNames();

with

var names = jsObjectKeys(nativeHeaders);

This solves my issue calling forEach both in the unit test and in my firebase function.
I guess it is not sufficient as there are more places where nativeResponse is used and I'm not sure why. Let me know if you would accept a PR or whether you plan to make more changes in this area

[pulyaevskiy]

Hi @alextekartik !

Feel free to submit a PR with your fix, I looked at the changes and it looks good to me.

For more background: the HttpHeaders base class is used in both HttpRequest and HttpResponse (I simply followed implementation from dart:io here), but there is a difference in that only response headers are modifiable, while request headers are read-only.

This is why all mutation methods (clear, add and all the setters) use nativeResponse and call _checkMutable which guards from mutating headers if:

(a) they are request headers (nativeResponse == null)
(b) they are response headers but the response has been sent to the client already

The forEach method does not modify any headers so it was my mistake to put nativeResponse.getHeaderNames() call in there. So your fix is appropriate.

Also, thanks for taking time to submit this issue, contributions and bug reports are highly appreciated.

P.S: I suspect you are but just in case: make sure you use all latest 0.1.0-beta versions for node_interop and firebase_functions. These have a lot more functionality and I'm making an effort to get public API in a better shape there.

[alextekartik]

Thanks for the fast response and good explanation. I submitted the PR, and yes I use your latest version (beta) in my dependency_overrides. Really good stuff you are doing here. I'm actually trying to have a light mock version of the firebase functions to debug cloud functions natively and then safely use the node version when deploying. Maybe having DDC supports for node build would help in the future. Thansk again