Skip to content

Commit

Permalink
fix: don't fail if ~/.config doesn't exist (#1428)
Browse files Browse the repository at this point in the history 
* fix: don't fail if ~/.config doesn't exist

* linted files

* refactored if statement

* refactored tests

* refactored code

* log statments to debug remote failing test

* log statments to debug remote failing test

* log statments to debug remote failing test

* log statments to debug remote failing test

* updated test to ensure directory exists

* removed logging statements

* linted files

* refactored code

* added comments suggested by @stephenplusplus

* test updated from diff https://gist.github.com/stephenplusplus/dccf237542b7a702ebff90f03f2a557a provided by @stephenplusplus

* removed unused dependency
  • Loading branch information
@shaffeeullah
shaffeeullah committed Apr 1, 2021
1 parent bd9ef18 commit 3cfaba1
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 52 deletions.
46 changes: 35 additions & 11 deletions src/file.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1797,13 +1797,41 @@ class File extends ServiceObject<File> {
return;
}

// Same as configstore:
// The logic below attempts to mimic the resumable upload library,
// gcs-resumable-upload. That library requires a writable configuration
// directory in order to work. If we wait for that library to discover any
// issues, we've already started a resumable upload which is difficult to back
// out of. We want to catch any errors first, so we can choose a simple, non-
// resumable upload instead.

// Same as configstore (used by gcs-resumable-upload):
// https://github.com/yeoman/configstore/blob/f09f067e50e6a636cfc648a6fc36a522062bd49d/index.js#L11
const configDir = xdgBasedir.config || os.tmpdir();

fs.access(configDir, fs.constants.W_OK, err => {
if (err) {
fs.access(configDir, fs.constants.W_OK, accessErr => {
if (!accessErr) {
// A configuration directory exists, and it's writable. gcs-resumable-upload
// should have everything it needs to work.
this.startResumableUpload_(fileWriteStream, options);
return;
}

// The configuration directory is either not writable, or it doesn't exist.
// gcs-resumable-upload will attempt to create it for the user, but we'll try
// it now to confirm that it won't have any issues. That way, if we catch the
// issue before we start the resumable upload, we can instead start a simple
// upload.
fs.mkdir(configDir, {mode: 0o0700}, err => {
if (!err) {
// We successfully created a configuration directory that
// gcs-resumable-upload will use.
this.startResumableUpload_(fileWriteStream, options);
return;
}

if (options.resumable) {
// The user wanted a resumable upload, but we couldn't create a
// configuration directory, which means gcs-resumable-upload will fail.
const error = new ResumableUploadError(
[
'A resumable upload could not be performed. The directory,',
Expand All @@ -1812,15 +1840,11 @@ class File extends ServiceObject<File> {
].join(' ')
);
stream.destroy(error);
return;
} else {
// The user didn't care, resumable or not. Fall back to simple upload.
this.startSimpleUpload_(fileWriteStream, options);
}

// User didn't care, resumable or not. Fall back to simple upload.
this.startSimpleUpload_(fileWriteStream, options);
return;
}

this.startResumableUpload_(fileWriteStream, options);
});
});
});

Expand Down
114 changes: 73 additions & 41 deletions test/file.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,21 +19,23 @@ import {
ServiceObjectConfig,
util,
} from '@google-cloud/common';
import {describe, it, before, beforeEach, afterEach} from 'mocha';
import {PromisifyAllOptions} from '@google-cloud/promisify';
import {Readable, PassThrough, Stream, Duplex, Transform} from 'stream';
import * as assert from 'assert';
import {describe, it, before, beforeEach, afterEach} from 'mocha';
import * as crypto from 'crypto';
import * as dateFormat from 'date-and-time';
import * as duplexify from 'duplexify';
import * as extend from 'extend';
import * as fs from 'fs';
import * as resumableUpload from 'gcs-resumable-upload';
import * as gaxios from 'gaxios';
import * as os from 'os';
import * as path from 'path';
import * as proxyquire from 'proxyquire';
import * as resumableUpload from 'gcs-resumable-upload';
import * as sinon from 'sinon';
import {Readable, PassThrough, Stream, Duplex, Transform} from 'stream';
import * as tmp from 'tmp';
import * as zlib from 'zlib';
import * as gaxios from 'gaxios';

import {
Bucket,
Expand Down Expand Up @@ -1859,57 +1861,87 @@ describe('File', () => {
file.createWriteStream({resumable: true}).write('data');
});

it('should fail if resumable requested but not writable', done => {
const error = new Error('Error.');
describe('config directory does not exist', () => {
const CONFIG_DIR = path.join(os.tmpdir(), `/fake-xdg-dir/${Date.now()}`);

Object.assign(fakeFs, {
access(dir: {}, check: {}, callback: Function) {
callback(error);
},
beforeEach(() => {
xdgConfigOverride = CONFIG_DIR;
fakeFs.access = fsCached.access;
});

const writable = file.createWriteStream({resumable: true});

writable.on('error', (err: Error) => {
assert.notStrictEqual(err, error);
it('should attempt to create the config directory', done => {
Object.assign(fakeFs, {
mkdir(dir: string, options: {}) {
assert.strictEqual(dir, CONFIG_DIR);
assert.deepStrictEqual(options, {mode: 0o0700});
done();
},
});

assert.strictEqual(err.name, 'ResumableUploadError');
const writable = file.createWriteStream({resumable: true});
writable.write('data');
});

const configDir = xdgBasedirCached.config;
it('should start a resumable upload if config directory created successfully', done => {
Object.assign(fakeFs, {
mkdir(dir: string, options: {}, callback: Function) {
callback();
},
});

assert.strictEqual(
err.message,
[
'A resumable upload could not be performed. The directory,',
`${configDir}, is not writable. You may try another upload,`,
'this time setting `options.resumable` to `false`.',
].join(' ')
);
file.startResumableUpload_ = () => {
// If no error is thrown here, we know the request completed successfully.
done();
};

done();
file.createWriteStream().write('data');
});

writable.write('data');
});
it('should return error if resumable was requested, but a config directory could not be created', done => {
Object.assign(fakeFs, {
mkdir(dir: string, options: {}, callback: Function) {
callback(new Error());
},
});

it('should fall back to simple if not writable', done => {
const options = {
metadata: METADATA,
customValue: true,
};
const writable = file.createWriteStream({resumable: true});

file.startSimpleUpload_ = (stream: {}, options_: {}) => {
assert.deepStrictEqual(options_, options);
done();
};
writable.on('error', (err: Error) => {
assert.strictEqual(err.name, 'ResumableUploadError');
assert.strictEqual(
err.message,
[
'A resumable upload could not be performed. The directory,',
`${CONFIG_DIR}, is not writable. You may try another upload,`,
'this time setting `options.resumable` to `false`.',
].join(' ')
);

Object.assign(fakeFs, {
access(dir: {}, check: {}, callback: Function) {
callback(new Error('Error.'));
},
done();
});

writable.write('data');
});

file.createWriteStream(options).write('data');
it('should fallback to a simple upload if the config directory could not be created', done => {
const options = {
metadata: METADATA,
customValue: true,
};

Object.assign(fakeFs, {
mkdir(dir: string, options: {}, callback: Function) {
callback(new Error());
},
});

file.startSimpleUpload_ = (stream: Stream, _options: {}) => {
assert.deepStrictEqual(_options, options);
done();
};

file.createWriteStream(options).write('data');
});
});

it('should default to a resumable upload', done => {
Expand Down

0 comments on commit 3cfaba1

Please sign in to comment.