Handle HTTP errors in DevSupportManager. #4880
Conversation
JunielKatarn
added
the
AutoMerge
|
Hello @JunielKatarn! Because this pull request has the Do note that I've been instructed to only help merge pull requests of this repository that have been opened for at least 10 hours. No worries though, I will be back when the time is right! 😉 p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
| connect(socket, resolveResult, ec); | ||
| if (ec) { | ||
| m_exceptionCaught = true; | ||
| return R"({"error:")"s + ec.message() + R"("})"s; |
There was a problem hiding this comment.
return [](start = 4, length = 6)
It's strange to return string as error. Why not just have m_errorString? Or some other error pattern?
There was a problem hiding this comment.
I disagree. This pattern is similar to an HRESULT or an HTTP code. There is always a return value, but it can be an error type.
Adding an instance member would be unnecessary, if error conditions are indicated by m_exceptionCaught. Only GetJavaScriptFromServer is meant to return a string. If this function fails, we may want a way to indicate the error details.
There was a problem hiding this comment.
See OInstance, which consumes the DevSupportManager.
I'd say so, but this would require changing |
| @@ -0,0 +1,8 @@ | |||
| { | |||
| "type": "none", | |||
There was a problem hiding this comment.
FYI, none means Beachball won't publish a build for this or include it in its changelog. This should be prerelease.
There was a problem hiding this comment.
I was on the fence on this one.
When this change is backported to 0.60-stable, I do plan to generate a new version.
for the master branch, didn't think it was worth it.
What criteria would you suggest?
Any change we want reflected on any branch as a new package version should be prerelease?
There was a problem hiding this comment.
Basically the only time we want to use “none” is when the change doesn’t impact the package (e.g. changing CI). We switched the master branch to a nightly build to reduce version spam, so you shouldn’t need to worry about this making a new build. Having the commit in a change log is nice though, and we would probably want to publish this if it was the only thing committed that day.
* improve treedump capabilities * Reenable V8 for desktop projects (#4840) * Delay load ChakraCore.dll * Change files * clean up unneeded project references * Overwrite property after React.Cpp.props gets included * Change files * change file * Fix WinUI3 * Reverse the conditional logic * Revert some unnecessary changes. Consolidate the preprocessor defines in the props Co-authored-by: tudorm <[email protected]> * applying package updates ***NO_CI*** * update masters * Improve run_wdio to print out failed tests (#4843) * Improve run_wdio to print out failed tests * Fire onLoad event when a bitmap image is opened (#4750) * Fire onLoad event when a bitmap image is opeed * Change files * add imageFailed * remove firing of onload events in the createImageBrush block so that load events don't fire twice. * Expose ability for apps to provide their own RedBox implementation (#4786) * Expose ability for apps to provide their own RedBox implementation * Change files * minor changes * Code review feedback * minor fix * format fixes * minor change * minor fix * minor fix * minor fix * Bump fp-ts from 2.5.4 to 2.6.0 (#4871) Bumps [fp-ts](https://github.com/gcanti/fp-ts) from 2.5.4 to 2.6.0. - [Release notes](https://github.com/gcanti/fp-ts/releases) - [Changelog](https://github.com/gcanti/fp-ts/blob/master/CHANGELOG.md) - [Commits](gcanti/fp-ts@2.5.4...2.6.0) Signed-off-by: dependabot-preview[bot] <[email protected]> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> * applying package updates ***NO_CI*** * Fixed ReactContext copy/move semantic (#4870) * Fixed ReactContext copy/move semantic * Change files * Fixed formatting * Test to show how to use a delegate in ReactPropertyBag * Add ReactNativeHost to Win32 C++/WinRT ABI (#4848) * create MS.ReactNative.IntegrationTests project * RNHost activation succeeds * update * update * Change files * add JS function call test * Change files * fix formatting * submit current state * ReactNativeHost activation test succeeds * Change files * account for property bag addition * sync, address code review feedback * Use spec file for DevSettings NativeModule (#4873) * Use spec file for DevSettings NativeModule * Change files * minor change * Handle HTTP errors in DevSupportManager. (#4880) * Handle HTTP connect exceptions in DevSupportManager * Change files * applying package updates ***NO_CI*** * Allow storing non-WinRT types in ReactPropertyBag (#4884) * Allow storing non-WinRT types in ReactPropertyBag * Change files * Addressed PR feedback * Use DispatcherQueue instead of CoreDispatcher for Mso::DispatchQueue (#4877) * Use DispatcherQueue instead of CoreDispatcher for Mso::DispatchQueue * Change files * Removed 'Thread' from some new API names. * Removed extra 'private' section in AppearanceModule * Temporary: use CoreDispatcher to check the E2E test * Avoid using DispatcherQueue::HasThreadAccess in earlier Windows versions. * RNW dependencies (#4876) * add rnw-dependencies.ps1 * Change files * --sln * run in CI * print * desktop appium is optional * use agent directory * 15GB is a ballpark estimate * optional space * . * support running from web (#4892) * Bump typescript from 3.8.3 to 3.9.2 (#4890) * Bump lerna from 3.20.2 to 3.21.0 (#4889) * BugFix: fix default tabindex stomping over acceptsKeyboardFocus (#4893) * Change files * revert dfc57fc * set default tab index to -1, the hope is to be in line with xbox assumption * xbox doesn't seem to rely on this, changing default back to match xaml default * Update e2e testing doc with CI debugging info (#4897) * add instructions for CI debugging for e2e test app * point e2e readme to e2e testing doc * Update rnw-dependencies (#4894) * don't exit the powershell session and pause for keyboard input if interactive * Change files * enable Switch (fixed 4596) * improve treedump capabilities * update masters * enable Switch (fixed 4596) * protect against exceptions in run_wdio * add another try block * update e2e testing and masters, fixes 4680 * publish wdio report and fix run_wdio typo bug * TreeDump should ignore collapsed object elements in an array * add info about run_wdio Co-authored-by: tudorms <[email protected]> Co-authored-by: tudorm <[email protected]> Co-authored-by: React-Native-Windows Bot <[email protected]> Co-authored-by: lamxdoan <[email protected]> Co-authored-by: Andrew Coates <[email protected]> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com> Co-authored-by: Vladimir Morozov <[email protected]> Co-authored-by: Andreas Eulitz <[email protected]> Co-authored-by: Julio César Rocha <[email protected]> Co-authored-by: kmelmon <[email protected]>
3 participants
DevSupportManagerusesboost::beast::httpfor HTTP calls, including downloading JS bundles.Errors such as resource/host unavailable are not being handled, thus crashing the host app.
Microsoft Reviewers: Open in CodeFlow