Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convert Buffer.byteLength to string before assigning to Content-Length. #956

Merged
merged 1 commit into from
Apr 11, 2018

Conversation

abernix
Copy link
Member

@abernix abernix commented Apr 10, 2018

This change seems necessary in order to meet the new type definitions for
res.setHeader which mandate that the argument be a string, or an array of
strings. Those type definitions were introduced via the @types/node@9
series of typing updates provided in (0), (1), (2).

With any luck, this will fix the failures being exhibited in CircleCI
tests (3) after we landed those typing updates.

…ngth`.

This change seems necessary in order to meet the new type definitions for
`res.setHeader` which mandate that the argument be a string, or an array of
strings.  Those type definitions were introduced via the `@types/node@9`
series of typing updates provided in [0], [1], [2].

With any luck, this will fix the failures being exhibited in CircleCI
tests[3] after we landed those typing updates.

[0]: #907
[1]: #927
[2]: #939
[3]: https://circleci.com/gh/apollographql/apollo-server/1587
@abernix
Copy link
Member Author

abernix commented Apr 10, 2018

All of the tests failed on the first run due to a separate typing failure in @types/koa due to its use of http2 which isn't available in Node 4, 6 or 8 (on 8 it is available but only if --expose-http2 is passed to node, which I don't believe we're doing).

First run: https://circleci.com/workflow-run/318777a7-cddf-4a79-90d9-a92a5da701a4

Subsequent run: https://circleci.com/workflow-run/a357627a-d90b-42cc-9b8b-6a6a09547d2b

I'm a bit perplexed as I fully expected the http2 usage to continue to fail until the types were rectified (or rolled back), not suddenly start working. The change in the types was introduced in DefinitelyTyped/DefinitelyTyped#24651, which should have surfaced as @types/koa@2.0.45 via #951.

@abernix
Copy link
Member Author

abernix commented Apr 10, 2018

Worth noting: It also failed in the Netlify build, so it wasn't isolated to CircleCI.


Update: The "retry" of the Netlify build worked fine too. 🤷‍♂️

@abernix abernix merged commit c2bba6b into master Apr 11, 2018
@abernix abernix deleted the abernix/convert-content-length-to-string branch August 10, 2018 16:30
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant