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

Use fetch-h2 by default with option to fallback to node-fetch #522

Merged
merged 5 commits into from
May 26, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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