Ensure log folder exists prior to initialization #583

awschristou · 2019-05-28T16:44:50Z

Types of changes

Bug fix (non-breaking change which fixes an issue)

Description

When the logger starts up, the logging folder is created prior to initialization. This eliminates a situation (#566) where the logging platform threw an error on initialization, which crashes extension activation.

To support this, I found out that electron apps throw an error if mkdir tries to recursively create more than one folder, so I added a drop-in clause that explicitly creates each folder one parent at a time.

Related Issue(s)

#566

Testing

Regular startup
started extension in remote WSL session
ran tests

Checklist

I have read the README document
I have read the CONTRIBUTING document
My code follows the code style of this project
I have added tests to cover my changes
All new and existing tests passed
A short description of the change has been added to the CHANGELOG

License

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

src/shared/filesystem.ts

mpiroc · 2019-05-28T18:17:12Z

src/shared/filesystem.ts

+    try {
+        await _mkdir(path, options)
+    } catch (err) {
+        // mkdir calls with recurse do not work as expected when called through electron.


Can you add a note that this will be be fixed if/when VS Code adopts Electron 5? We should monitor the issue and remove this workaround when it is resolved.

Definitely -- the provided link isn't intended for tracking, only for background info into the situation

mpiroc · 2019-05-28T18:18:16Z

src/shared/filesystem.ts

+        await access(path)
+
+        // path exists, nothing left to do
+        return


The return statement is redundant, as there is no code following it.

Thanks - I rearranged the code and didn't catch that before PR

src/shared/logger.ts

bryceitoc9 · 2019-05-28T19:35:59Z

src/shared/filesystem.ts

+    options: MakeDirectoryOptions,
+): Promise<void> {
+    try {
+        await access(path)


Would you ever hit this line? Looks like you'd only hit mkdirRecursive if the directory doesn't exist.

I started with a standalone function, but switched to the format you see in the PR. You're right, remove it

codecov-io · 2019-05-28T23:14:03Z

Codecov Report

Merging #583 into develop will increase coverage by 0.19%.
The diff coverage is 88.88%.

@@             Coverage Diff             @@
##           develop     #583      +/-   ##
===========================================
+ Coverage    55.55%   55.75%   +0.19%     
===========================================
  Files          115      115              
  Lines         4435     4459      +24     
  Branches       668      672       +4     
===========================================
+ Hits          2464     2486      +22     
- Misses        1971     1973       +2

Impacted Files	Coverage Δ
src/shared/filesystemUtilities.ts	`100% <100%> (ø)`	⬆️
src/shared/logger.ts	`90.32% <85.71%> (+0.66%)`	⬆️
src/shared/filesystem.ts	`93.1% <89.47%> (-6.9%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9f6da6a...9272617. Read the comment docs.

…evelop.

…y. (#599) * Initial codelens implementation for C# (#477) * Open sam template upon sam init (#481) * C# run command (#488) Added C# run command with some tests and some refactoring to the localLambdaRunner and pythonCodeRunner files. Additional work deemed out-of-scope for this feature can be found here: #507 * Prepare for .NET Core Debug implemention. (#513) * Removing unused runtimes (#535) * Implement "Debug Locally" code lens for .NET Core. (#552) * Port changes from develop for eb07933..9c3d7c3. * Fix npm audit issues. * Port 'Switch away from Terminal to using Debug Console and Output tabs for local debug and run (#493)' from develop. * Port 'Added AWS logo with min retina size (#461) (#498)' from develop. * Port 'Handle port-waiting timeout; Catch Python debugging Errors (#494)' from develop. * Python Path Mappings now provide both drive letter casings for Windows (#495) * Port 'Explicitly show the Debug Console after each successful debugger attach (#496)' from develop. * Port 'Prevent AWS Explorer context menu items from getting added to other extension/view menus (#506)' from develop * Port 'Design for operations against resources attached to code pipelines (#503)' from develop. * Port 'childProcess times out and dies if the debugger does not attach in time (#512)' from develop. * Port 'Remove lambda views for Policy and Configuration (#505)' from develop. * Port 'Normalizing telemetry (#534)' from develop. * Port 'SAM calls that use AWS resources now correctly use an AWS profile (#554)' from develop. * Port 'SAM CLI Validation Refactor (#555)' from develop. * Port 'Fixes issue introduced in f9816c1 that caused the AWS Explorer context menu to disappear (#560)' from develop * Port 'Added a help button to the sam init wizard's runtime selection step (#538)' from develop. * Port 'Metrics overrides for result and duration (#562)Metrics overrides for result and duration (#562)' from develop. * Port 'Added vs code task to run lint fixer (#561)' from develop. * Port 'SAM CLI Validation at the start of Create New SAM App flow (#563)' from develop. * Port 'Updated the message shown when SAM CLI is newer than valid range (#569)' from develop. * Port 'SAM CLI Validation at the start of Deploy SAM App flow (#570)' from develop. * Port 'Code Improvement: sam deploy calls (#571)' from develop. * Port 'Code Improvement: sam package calls (#572)' from develop. * Port 'Telemetry data includes active AWS account ID (if configured) (#557)' from develop. * Port 'Code Improvement: sam deploy workflow (#573)' from develop. * Port 'Code Improvement: Error handling on sam cli calls (#574)' from develop. * Port 'Extension throws an error if the toolkitClientBuilder is not initialized before initial credential validation (#577)' from develop. * Port 'addUserDataToContext throws an error if a profile's credentials are nonexistent (#579)' from develop. * Port 'Ensure log folder exists prior to initialization (#583)' from develop. * Port 'Adding help buttons to deploy workflow (#584)' from develop. * Port 'Credentials are now passed to STS client correctly on validation. (#585)' from develop * Port 'Add prettier formatter and git hook (#578)' from develop. * Fix WAIT_FOR_DEBUGGER_MESSAGE for .NET Core. * Disable prettier pre-commit hook, and fix linter issues introduced by prettier. * Fix dotnet debug install issues on Mac, Linux (#592) * Instead of installing VSDBG by manually invoking crossSpawn, use existing ChildProcess utility class. * Simplify code by unconditionally installing debugger each time - the script is smart enough to determine if an install is needed. * DockerContext outputs stdout to output channel * Fix installing debugger on non-windows platforms * Fix issue that caused templates.json to be ignored when invoking .NET Core functions. (#593) * Address feedback from PR.

Due to a series of misteps, we are squashing this entire feature branch into develop. 1. We allowed `feature/dotnet` to drift too far from `develop`, which required significant manual conflict resolution to resolve. 2. We should have squashed the conflict-resolution history *before* merging develop, but we did not. Therefore in order to squash the pre-merge commits, we would need to recreate the merge and re-resolve conflicts. This would reduce our confidence in the quality of the branch. In the future: 1. We will regularly merge into feature branches to prevent drift. 2. If we do encounter a situation that involves manually porting, we will squash the ports *before* merging develop. * Initial codelens implementation for C# (#477) * Open sam template upon sam init (#481) * C# run command (#488) Added C# run command with some tests and some refactoring to the localLambdaRunner and pythonCodeRunner files. Additional work deemed out-of-scope for this feature can be found here: #507 * Prepare for .NET Core Debug implemention. (#513) * Removing unused runtimes (#535) * Implement "Debug Locally" code lens for .NET Core. (#552) * Port changes from develop for eb07933..9c3d7c3. * Fix npm audit issues. * Port 'Switch away from Terminal to using Debug Console and Output tabs for local debug and run (#493)' from develop. * Port 'Added AWS logo with min retina size (#461) (#498)' from develop. * Port 'Handle port-waiting timeout; Catch Python debugging Errors (#494)' from develop. * Python Path Mappings now provide both drive letter casings for Windows (#495) * Port 'Explicitly show the Debug Console after each successful debugger attach (#496)' from develop. * Port 'Prevent AWS Explorer context menu items from getting added to other extension/view menus (#506)' from develop * Port 'Design for operations against resources attached to code pipelines (#503)' from develop. * Port 'childProcess times out and dies if the debugger does not attach in time (#512)' from develop. * Port 'Remove lambda views for Policy and Configuration (#505)' from develop. * Port 'Normalizing telemetry (#534)' from develop. * Port 'SAM calls that use AWS resources now correctly use an AWS profile (#554)' from develop. * Port 'SAM CLI Validation Refactor (#555)' from develop. * Port 'Fixes issue introduced in f9816c1 that caused the AWS Explorer context menu to disappear (#560)' from develop * Port 'Added a help button to the sam init wizard's runtime selection step (#538)' from develop. * Port 'Metrics overrides for result and duration (#562)Metrics overrides for result and duration (#562)' from develop. * Port 'Added vs code task to run lint fixer (#561)' from develop. * Port 'SAM CLI Validation at the start of Create New SAM App flow (#563)' from develop. * Port 'Updated the message shown when SAM CLI is newer than valid range (#569)' from develop. * Port 'SAM CLI Validation at the start of Deploy SAM App flow (#570)' from develop. * Port 'Code Improvement: sam deploy calls (#571)' from develop. * Port 'Code Improvement: sam package calls (#572)' from develop. * Port 'Telemetry data includes active AWS account ID (if configured) (#557)' from develop. * Port 'Code Improvement: sam deploy workflow (#573)' from develop. * Port 'Code Improvement: Error handling on sam cli calls (#574)' from develop. * Port 'Extension throws an error if the toolkitClientBuilder is not initialized before initial credential validation (#577)' from develop. * Port 'addUserDataToContext throws an error if a profile's credentials are nonexistent (#579)' from develop. * Port 'Ensure log folder exists prior to initialization (#583)' from develop. * Port 'Adding help buttons to deploy workflow (#584)' from develop. * Port 'Credentials are now passed to STS client correctly on validation. (#585)' from develop * Port 'Add prettier formatter and git hook (#578)' from develop. * Fix WAIT_FOR_DEBUGGER_MESSAGE for .NET Core. * Disable prettier pre-commit hook, and fix linter issues introduced by prettier. * Fix dotnet debug install issues on Mac, Linux (#592) * Instead of installing VSDBG by manually invoking crossSpawn, use existing ChildProcess utility class. * Simplify code by unconditionally installing debugger each time - the script is smart enough to determine if an install is needed. * DockerContext outputs stdout to output channel * Fix installing debugger on non-windows platforms * Fix issue that caused templates.json to be ignored when invoking .NET Core functions. (#593) * Address feedback from PR.

awschristou added 3 commits May 24, 2019 12:52

Fix case where logging is unable to create the log folder

190b0c7

mkdir now makes an attempt to recursively create missing folders

4372f8e

Lint fix

e020b61

awschristou added this to the Sprint 22 milestone May 28, 2019

awschristou requested review from mpiroc and bryceitoc9 May 28, 2019 16:44

awschristou mentioned this pull request May 28, 2019

Toolkit doesn't work when 'WSL - Installed' #566

Closed

mpiroc suggested changes May 28, 2019

View reviewed changes

bryceitoc9 reviewed May 28, 2019

View reviewed changes

Feedback

9272617

Merge branch 'develop' into awschristou/log-init-mkdir

47756bf

bryceitoc9 approved these changes May 28, 2019

View reviewed changes

Merge branch 'develop' into awschristou/log-init-mkdir

8ed8339

mpiroc approved these changes May 29, 2019

View reviewed changes

awschristou merged commit 9a90827 into develop May 29, 2019

awschristou deleted the awschristou/log-init-mkdir branch May 29, 2019 00:04

mpiroc added a commit that referenced this pull request May 30, 2019

Port 'Ensure log folder exists prior to initialization (#583)' from d…

14519df

…evelop.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ensure log folder exists prior to initialization #583

Ensure log folder exists prior to initialization #583

awschristou commented May 28, 2019

mpiroc May 28, 2019

awschristou May 28, 2019

mpiroc May 28, 2019

awschristou May 28, 2019

bryceitoc9 May 28, 2019

awschristou May 28, 2019

codecov-io commented May 28, 2019

Ensure log folder exists prior to initialization #583

Ensure log folder exists prior to initialization #583

Conversation

awschristou commented May 28, 2019

Types of changes

Description

Related Issue(s)

Testing

Checklist

License

mpiroc May 28, 2019

Choose a reason for hiding this comment

awschristou May 28, 2019

Choose a reason for hiding this comment

mpiroc May 28, 2019

Choose a reason for hiding this comment

awschristou May 28, 2019

Choose a reason for hiding this comment

bryceitoc9 May 28, 2019

Choose a reason for hiding this comment

awschristou May 28, 2019

Choose a reason for hiding this comment

codecov-io commented May 28, 2019

Codecov Report