From 87f64c4535b78cb38b7263cb0ad56da908964d85 Mon Sep 17 00:00:00 2001 From: Pritesh Nandgaonkar Date: Thu, 17 Jan 2019 16:08:49 -0800 Subject: [PATCH] Send crash notification when flipper cpp exceptions are suppressed Summary: This diff adds support to send crash notification whenever the cpp exception of Flipper is suppressed. Also updated the tests regarding the same Reviewed By: jknoxville Differential Revision: D13635822 fbshipit-source-id: 01e4a57c391476e5b044e64946d337cb4582a527 --- src/Client.js | 40 ++++++++++++++ src/plugins/crash_reporter/index.js | 2 +- xplat/Flipper/Flipper.podspec | 4 +- xplat/Flipper/FlipperClient.cpp | 47 ++++++++++++++--- xplat/Flipper/FlipperClient.h | 13 +++-- xplat/Flipper/utils/CallstackHelper.h | 63 +++++++++++++++++++++++ xplat/FlipperTests/FlipperClientTests.cpp | 17 +++--- 7 files changed, 164 insertions(+), 22 deletions(-) create mode 100644 xplat/Flipper/utils/CallstackHelper.h diff --git a/src/Client.js b/src/Client.js index f9214d0ce..97dd80a09 100644 --- a/src/Client.js +++ b/src/Client.js @@ -27,8 +27,43 @@ export type ClientQuery = {| device_id: string, |}; +type ErrorType = {message: string, stacktrace: string, name: string}; type RequestMetadata = {method: string, id: number, params: ?Object}; +const handleError = (store: Store, deviceSerial: ?string, error: ErrorType) => { + const crashReporterPlugin = store + .getState() + .plugins.devicePlugins.get('CrashReporter'); + if (!crashReporterPlugin) { + return; + } + + const pluginKey = `${deviceSerial || ''}#CrashReporter`; + + const persistedState = { + ...crashReporterPlugin.defaultPersistedState, + ...store.getState().pluginStates[pluginKey], + }; + // $FlowFixMe: We checked persistedStateReducer exists + const newPluginState = crashReporterPlugin.persistedStateReducer( + persistedState, + 'flipper-crash-report', + { + name: error.name, + reason: error.message, + callstack: error.stacktrace, + }, + ); + if (persistedState !== newPluginState) { + store.dispatch( + setPluginState({ + pluginKey, + state: newPluginState, + }), + ); + } +}; + export default class Client extends EventEmitter { constructor( id: string, @@ -162,6 +197,7 @@ export default class Client extends EventEmitter { }: ${error.message} + \nDevice Stack Trace: ${error.stacktrace}`, 'deviceError', ); + handleError(this.store, this.getDevice()?.serial, error); } else if (method === 'refreshPlugins') { this.refreshPlugins(); } else if (method === 'execute') { @@ -226,6 +262,10 @@ export default class Client extends EventEmitter { callbacks.resolve(data.success); } else if (data.error) { callbacks.reject(data.error); + const {error} = data; + if (error) { + handleError(this.store, this.getDevice()?.serial, error); + } } else { // ??? } diff --git a/src/plugins/crash_reporter/index.js b/src/plugins/crash_reporter/index.js index 98ecdc1e0..00c5b5e6b 100644 --- a/src/plugins/crash_reporter/index.js +++ b/src/plugins/crash_reporter/index.js @@ -279,7 +279,7 @@ export default class CrashReporterPlugin extends FlipperDevicePlugin< method: string, payload: Object, ): PersistedState => { - if (method === 'crash-report') { + if (method === 'crash-report' || method === 'flipper-crash-report') { CrashReporterPlugin.notificationID++; const mergedState: PersistedState = { crashes: persistedState.crashes.concat([ diff --git a/xplat/Flipper/Flipper.podspec b/xplat/Flipper/Flipper.podspec index 52e23b046..0bc67490b 100644 --- a/xplat/Flipper/Flipper.podspec +++ b/xplat/Flipper/Flipper.podspec @@ -9,8 +9,8 @@ Pod::Spec.new do |spec| spec.source = { :git => 'https://github.com/facebook/Sonar.git', :tag => 'v'+flipperkit_version } spec.module_name = 'Flipper' - spec.public_header_files = 'xplat/Flipper/*.h' - spec.source_files = 'xplat/Flipper/*.{h,cpp,m,mm}' + spec.public_header_files = 'xplat/Flipper/*.h','xplat/utils/*.h' + spec.source_files = 'xplat/Flipper/*.{h,cpp,m,mm}','xplat/Flipper/utils/*.{h,cpp,m,mm}' spec.libraries = "stdc++" spec.dependency 'Folly', '~>1.1' spec.dependency 'RSocket', '~>0.10' diff --git a/xplat/Flipper/FlipperClient.cpp b/xplat/Flipper/FlipperClient.cpp index fc52f9b3e..aec308f8a 100644 --- a/xplat/Flipper/FlipperClient.cpp +++ b/xplat/Flipper/FlipperClient.cpp @@ -1,9 +1,8 @@ -/* - * Copyright (c) Facebook, Inc. - * - * This source code is licensed under the MIT license found in the LICENSE - * file in the root directory of this source tree. +/** + * Copyright (c) Facebook, Inc. and its affiliates. * + * This source code is licensed under the MIT license found in the LICENSE + * file in the root directory of this source tree. */ #include "FlipperClient.h" #include @@ -17,6 +16,12 @@ #include "FlipperState.h" #include "FlipperStep.h" #include "Log.h" +#if __ANDROID__ +#include "utils/CallstackHelper.h" +#endif +#if __APPLE__ +#include +#endif #if FB_SONARKIT_ENABLED @@ -222,6 +227,31 @@ void FlipperClient::onMessageReceived(const dynamic& message) { responder->error(response); }); } +std::string FlipperClient::callstack() { +#if __APPLE__ + // For some iOS apps, __Unwind_Backtrace symbol wasn't found in sandcastle + // builds, thus, for iOS apps, using backtrace c function. + void* callstack[2048]; + int frames = backtrace(callstack, 2048); + char** strs = backtrace_symbols(callstack, frames); + std::string output = ""; + for (int i = 0; i < frames; ++i) { + output.append(strs[i]); + output.append("\n"); + } + return output; +#elif __ANDROID__ + const size_t max = 2048; + void* buffer[max]; + std::ostringstream oss; + + dumpBacktrace(oss, buffer, captureBacktrace(buffer, max)); + std::string output = std::string(oss.str().c_str()); + return output; +#else + return ""; +#endif +} void FlipperClient::performAndReportError(const std::function& func) { #if FLIPPER_ENABLE_CRASH // To debug the stack trace and an exception turn on the compiler flag @@ -231,9 +261,12 @@ void FlipperClient::performAndReportError(const std::function& func) { try { func(); } catch (std::exception& e) { - dynamic message = dynamic::object( - "error", dynamic::object("message", e.what())("stacktrace", "")); if (connected_) { + std::string callstack = this->callstack(); + dynamic message = dynamic::object( + "error", + dynamic::object("message", e.what())("stacktrace", callstack)( + "name", e.what())); socket_->sendMessage(message); } else { log("Error: " + std::string(e.what())); diff --git a/xplat/Flipper/FlipperClient.h b/xplat/Flipper/FlipperClient.h index 4e4c7dbb4..a5eb9f87b 100644 --- a/xplat/Flipper/FlipperClient.h +++ b/xplat/Flipper/FlipperClient.h @@ -1,9 +1,8 @@ -/* - * Copyright (c) Facebook, Inc. - * - * This source code is licensed under the MIT license found in the LICENSE - * file in the root directory of this source tree. +/** + * Copyright (c) Facebook, Inc. and its affiliates. * + * This source code is licensed under the MIT license found in the LICENSE + * file in the root directory of this source tree. */ #pragma once @@ -46,6 +45,9 @@ class FlipperClient : public FlipperConnectionManager::Callbacks { : socket_(std::move(socket)), flipperState_(state) { auto step = flipperState_->start("Create client"); socket_->setCallbacks(this); + auto& conn = connections_["flipper-crash-report"]; + conn = std::make_shared( + socket_.get(), "flipper-crash-report"); step->complete(); } @@ -105,6 +107,7 @@ class FlipperClient : public FlipperConnectionManager::Callbacks { void disconnect(std::shared_ptr plugin); void startBackgroundPlugins(); + std::string callstack(); }; } // namespace flipper diff --git a/xplat/Flipper/utils/CallstackHelper.h b/xplat/Flipper/utils/CallstackHelper.h new file mode 100644 index 000000000..df8b09425 --- /dev/null +++ b/xplat/Flipper/utils/CallstackHelper.h @@ -0,0 +1,63 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the LICENSE + * file in the root directory of this source tree. + */ +#if FB_SONARKIT_ENABLED + +#include +#include +#include + +namespace facebook { +namespace flipper { +// TODO: T39093653, Replace the backtrace implementation with folly +// implementation. Didn't use the backtrace() c function as it was not found in +// NDK. +struct BacktraceState { + void** current; + void** end; +}; + +static _Unwind_Reason_Code unwindCallback( + struct _Unwind_Context* context, + void* arg) { + BacktraceState* state = static_cast(arg); + uintptr_t pc = _Unwind_GetIP(context); + if (pc) { + if (state->current == state->end) { + return _URC_END_OF_STACK; + } else { + *state->current++ = reinterpret_cast(pc); + } + } + return _URC_NO_REASON; +} + +static size_t captureBacktrace(void** buffer, size_t max) { + BacktraceState state = {buffer, buffer + max}; + _Unwind_Backtrace(unwindCallback, &state); + + return state.current - buffer; +} + +static void dumpBacktrace(std::ostream& os, void** buffer, size_t count) { + for (size_t idx = 0; idx < count; ++idx) { + const void* addr = buffer[idx]; + const char* symbol = ""; + + Dl_info info; + if (dladdr(addr, &info) && info.dli_sname) { + symbol = info.dli_sname; + } + + os << " #" << std::setw(2) << idx << ": " << addr << " " << symbol + << "\n"; + } +} + +} // namespace flipper +} // namespace facebook + +#endif diff --git a/xplat/FlipperTests/FlipperClientTests.cpp b/xplat/FlipperTests/FlipperClientTests.cpp index 1d7aecf97..9acbbeabe 100644 --- a/xplat/FlipperTests/FlipperClientTests.cpp +++ b/xplat/FlipperTests/FlipperClientTests.cpp @@ -1,9 +1,8 @@ -/* - * Copyright (c) Facebook, Inc. - * - * This source code is licensed under the MIT license found in the LICENSE - * file in the root directory of this source tree. +/** + * Copyright (c) Facebook, Inc. and its affiliates. * + * This source code is licensed under the MIT license found in the LICENSE + * file in the root directory of this source tree. */ #include #include @@ -282,10 +281,12 @@ TEST(FlipperClientTests, testExceptionUnknownPlugin) { dynamic messageInit = dynamic::object("method", "init")( "params", dynamic::object("plugin", "Unknown")); socket->callbacks->onMessageReceived(messageInit); - EXPECT_EQ( socket->messages.back()["error"]["message"], "plugin Unknown not found for method init"); + EXPECT_EQ( + socket->messages.back()["error"]["name"], + "plugin Unknown not found for method init"); } TEST(FlipperClientTests, testExceptionUnknownApi) { @@ -297,10 +298,12 @@ TEST(FlipperClientTests, testExceptionUnknownApi) { dynamic messageInit = dynamic::object("method", "execute")( "params", dynamic::object("api", "Unknown")); socket->callbacks->onMessageReceived(messageInit); - EXPECT_EQ( socket->messages.back()["error"]["message"], "connection Unknown not found for method execute"); + EXPECT_EQ( + socket->messages.back()["error"]["name"], + "connection Unknown not found for method execute"); } TEST(FlipperClientTests, testBackgroundPluginActivated) {