Skip to content

Commit

Permalink
Use fetch-h2 by default with option to fallback to node-fetch (#522)
Browse files Browse the repository at this point in the history
* Replaces `node-fetch` with `fetch-h2` on `core`

- `node-fetch` doesn't support HTTP/2 and has had an issue for
  support open since 2017: node-fetch/node-fetch#342
- `fetch-h2` supports HTTP1.1 and HTTP/2 with upgrade.
- Defaults to using `fetch-h2`.
- Allows optionally using `node-fetch`.
- Centralises calls to `fetch` in `FetchUtils`.
  • Loading branch information
andreban authored May 26, 2021
1 parent d8019d0 commit cbab770
Show file tree
Hide file tree
Showing 10 changed files with 209 additions and 61 deletions.
5 changes: 5 additions & 0 deletions packages/cli/src/lib/Cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {updateConfig} from './cmds/updateConfig';
import {doctor} from './cmds/doctor';
import {merge} from './cmds/merge';
import {fingerprint} from './cmds/fingerprint';
import {fetchUtils} from '@bubblewrap/core';

export class Cli {
async run(args: string[]): Promise<boolean> {
Expand All @@ -38,6 +39,10 @@ export class Cli {
' Node.js version 12 or above is required to run bubblewrap.');
}
const parsedArgs = minimist(args);
if (parsedArgs.fetchEngine &&
(parsedArgs.fetchEngine == 'node-fetch' || parsedArgs.fetchEngine == 'fetch-h2')) {
fetchUtils.setFetchEngine(parsedArgs.fetchEngine);
}

const config = await loadOrCreateConfig(undefined, undefined, parsedArgs.config);

Expand Down
4 changes: 2 additions & 2 deletions packages/cli/src/lib/Prompt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import {Result, util} from '@bubblewrap/core';
import {Result, fetchUtils} from '@bubblewrap/core';
import {Presets, Bar} from 'cli-progress';
import {green} from 'colors';
import * as inquirer from 'inquirer';
Expand Down Expand Up @@ -193,7 +193,7 @@ export class InquirerPrompt implements Prompt {
}, Presets.shades_classic);

progressBar.start(Math.round(totalSize / KILOBYTE_SIZE), 0);
await util.downloadFile(url, filename, (current, total) => {
await fetchUtils.downloadFile(url, filename, (current, total) => {
if (total > 0 && total !== totalSize) {
progressBar.setTotal(Math.round(total / KILOBYTE_SIZE));
totalSize = total;
Expand Down
90 changes: 87 additions & 3 deletions packages/core/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 3 additions & 2 deletions packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -36,16 +36,17 @@
"@types/lodash.template": "^4.5.0",
"@types/mime-types": "^2.1.0",
"@types/node": "^12.20.1",
"@types/node-fetch": "^2.5.8",
"@types/node-fetch": "^2.5.10",
"@types/tar": "^4.0.4",
"@types/valid-url": "^1.0.3",
"color": "^3.1.3",
"extract-zip": "^1.7.0",
"fetch-h2": "^2.5.1",
"inquirer": "^7.3.3",
"jimp": "^0.14.0",
"lodash.template": "^4.5.0",
"mime-types": "^2.1.28",
"node-fetch": "^2.6.0",
"node-fetch": "^2.6.1",
"svg2img": "^0.8.1",
"tar": "^6.1.0",
"valid-url": "^1.0.9"
Expand Down
2 changes: 2 additions & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,14 @@ import {TwaGenerator} from './lib/TwaGenerator';
import {DigitalAssetLinks} from './lib/DigitalAssetLinks';
import * as util from './lib/util';
import {Result} from './lib/Result';
import {fetchUtils} from './lib/FetchUtils';

export {
AndroidSdkTools,
BufferedLog,
Config,
DigitalAssetLinks,
fetchUtils,
Fingerprint,
GradleWrapper,
JarSigner,
Expand Down
93 changes: 93 additions & 0 deletions packages/core/src/lib/FetchUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
/*
* Copyright 2021 Google Inc. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import * as fs from 'fs';
import {fetch as fetchh2, Response as FetchH2Response} from 'fetch-h2';
import nodefetch, {Response as NodeFetchResponse} from 'node-fetch';

export type FetchEngine = 'node-fetch' | 'fetch-h2';
const DEFAULT_FETCH_ENGINE: FetchEngine = 'fetch-h2';

export type NodeFetchOrFetchH2Response = FetchH2Response | NodeFetchResponse;

class FetchUtils {
private fetchEngine = DEFAULT_FETCH_ENGINE;

setFetchEngine(newFetchEngine: FetchEngine): void {
this.fetchEngine = newFetchEngine;
}

async fetch(input: string): Promise<NodeFetchOrFetchH2Response> {
if (this.fetchEngine == 'node-fetch') {
return await nodefetch(input, {redirect: 'follow'});
} else {
return await fetchh2(input, {redirect: 'follow'});
}
}

/**
* Downloads a file from `url` and saves it to `path`. If a `progressCallback` function is passed, it
* will be invoked for every chunk received. If the value of `total` parameter is -1, it means we
* were unable to determine to total file size before starting the download.
*/
async downloadFile(
url: string,
path: string,
progressCallback?: (current: number, total: number) => void,
): Promise<void> {
let result;
let readableStream: NodeJS.ReadableStream;

if (this.fetchEngine === 'node-fetch') {
result = await nodefetch(url);
readableStream = result.body;
} else {
result = await fetchh2(url, {redirect: 'follow'});
readableStream = await result.readable();
}

// Try to determine the file size via the `Content-Length` header. This may not be available
// for all cases.
const contentLength = result.headers.get('content-length');
const fileSize = contentLength ? parseInt(contentLength) : -1;

const fileStream = fs.createWriteStream(path);
let received = 0;

await new Promise((resolve, reject) => {
readableStream.pipe(fileStream);

// Even though we're piping the chunks, we intercept them to check for the download progress.
if (progressCallback) {
readableStream.on('data', (chunk) => {
received = received + chunk.length;
progressCallback(received, fileSize);
});
}

readableStream.on('error', (err) => {
reject(err);
});

fileStream.on('finish', () => {
resolve();
});
});
}
}

const fetchUtils = new FetchUtils();
export {fetchUtils};
10 changes: 5 additions & 5 deletions packages/core/src/lib/ImageHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import * as path from 'path';
import * as fs from 'fs';
import * as Jimp from 'jimp';
import fetch from 'node-fetch';
import {fetchUtils} from './FetchUtils';
import Color = require('color');
import {promisify} from 'util';
import {svg2img} from './wrappers/svg2img';
Expand Down Expand Up @@ -109,7 +109,8 @@ export class ImageHelper {
* @returns an Object containing the original URL and the icon image data.
*/
async fetchIcon(iconUrl: string): Promise<Icon> {
const response = await fetch(iconUrl);
const response = await fetchUtils.fetch(iconUrl);

if (response.status !== 200) {
throw new Error(
`Failed to download icon ${iconUrl}. Responded with status ${response.status}`);
Expand All @@ -120,14 +121,13 @@ export class ImageHelper {
throw new Error(`Received icon "${iconUrl}" with invalid Content-Type.` +
` Responded with Content-Type "${contentType}"`);
}
let body;
body = await response.buffer();
let body = await response.arrayBuffer();
if (contentType.startsWith('image/svg')) {
body = await svg2img(iconUrl);
}
return {
url: iconUrl,
data: await Jimp.read(body),
data: await Jimp.read(Buffer.from(body)),
};
}
}
4 changes: 2 additions & 2 deletions packages/core/src/lib/TwaGenerator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import * as path from 'path';
import * as fs from 'fs';
import * as Color from 'color';
import fetch from 'node-fetch';
import {template} from 'lodash';
import {promisify} from 'util';
import {TwaManifest} from './TwaManifest';
Expand All @@ -26,6 +25,7 @@ import {Log} from './Log';
import {ImageHelper, IconDefinition} from './ImageHelper';
import {FeatureManager} from './features/FeatureManager';
import {rmdir, escapeJsonString, toAndroidScreenOrientation} from './util';
import {fetchUtils} from './FetchUtils';

const COPY_FILE_LIST = [
'settings.gradle',
Expand Down Expand Up @@ -266,7 +266,7 @@ export class TwaGenerator {
'Unable to write the Web Manifest. The TWA Manifest does not have a webManifestUrl');
}

const response = await fetch(twaManifest.webManifestUrl);
const response = await fetchUtils.fetch(twaManifest.webManifestUrl.toString());
if (response.status !== 200) {
throw new Error(`Failed to download Web Manifest ${twaManifest.webManifestUrl}.` +
`Responded with status ${response.status}`);
Expand Down
Loading

0 comments on commit cbab770

Please sign in to comment.