Skip to content
This repository has been archived by the owner on Dec 13, 2018. It is now read-only.

Commit

Permalink
add error message for logview
Browse files Browse the repository at this point in the history
Summary: add try/catch and log error so that metrics can be captured for thirft failure rates

Reviewed By: hansonw

Differential Revision: D9505463

fbshipit-source-id: e855fd1e36dfb03783636fbe14b0d20fad956d84
  • Loading branch information
cs01 authored and facebook-github-bot committed Aug 27, 2018
1 parent d05bb88 commit 6e8f6f4
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import fuse from 'fuse-bindings';
import thrift from 'thrift';
import LRU from 'lru-cache';
import RemoteFileSystemService from '../../../gen-nodejs/RemoteFileSystemService';
import ThriftFileSystemService from '../../../gen-nodejs/ThriftFileSystemService';

export class MountedFileSystem {
_root: string;
Expand Down Expand Up @@ -209,7 +209,7 @@ export class MountedFileSystem {
transport: thrift.TBufferedTransport(),
protocol: thrift.TBinaryProtocol(),
});
const client = thrift.createClient(RemoteFileSystemService, connection);
const client = thrift.createClient(ThriftFileSystemService, connection);
return Promise.resolve(
new MountedFileSystem(root, mountPath, client, connection),
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,13 @@ import {createThriftServer} from '../createThriftServer';
import thrift from 'thrift';
import * as portHelper from '../../../common/ports';

const RemoteFileSystemServiceHandler = jest.fn(function(root, watchman) {
const ThriftFileSystemServiceHandler = jest.fn(function(root, watchman) {
this._watcher = watchman;
});
const mockPort = 9090;

jest.mock(require.resolve('../../fs/ThriftFileSystemServiceHandler'), () => ({
RemoteFileSystemServiceHandler,
ThriftFileSystemServiceHandler,
}));

jest
Expand Down
22 changes: 15 additions & 7 deletions pkg/nuclide-remote-connection/lib/ServerConnection.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ import {
import electron from 'electron';

const logger = getLogger('nuclide-remote-connection');
const thriftRfsLogger = getLogger('thrift-rfs-server-connection');
const remote = electron.remote;
const ipc = electron.ipcRenderer;
const THRIFT_RFS_GK = 'nuclide_thrift_rfs';
Expand Down Expand Up @@ -571,7 +572,6 @@ export class ServerConnection {
return this._makeThriftRfsCall(propKey, args);
};
}
logger.error('Using legacy rfs for method: ', propKey);
return target[propKey];
},
};
Expand All @@ -585,12 +585,20 @@ export class ServerConnection {
return trackTimingSampled(
`file-system-service:${fsOperation}`,
async () => {
const thriftRfsClient = await getOrCreateRfsClientAdapter(
this.getBigDigClient(),
);
// $FlowFixMe: suppress 'indexer property is missing warning'
const method = thriftRfsClient[fsOperation];
return method.apply(thriftRfsClient, args);
try {
const thriftRfsClient = await getOrCreateRfsClientAdapter(
this.getBigDigClient(),
);
// $FlowFixMe: suppress 'indexer property is missing warning'
const method = thriftRfsClient[fsOperation];
return await method.apply(thriftRfsClient, args);
} catch (e) {
thriftRfsLogger.error(
`failed to run method ${fsOperation} from Thrift client`,
e,
);
throw e;
}
},
FILE_SYSTEM_PERFORMANCE_SAMPLE_RATE,
{serviceProvider: 'thrift'},
Expand Down

0 comments on commit 6e8f6f4

Please sign in to comment.