WIP: Ngrok Debugger Implementation #2032
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 13 commits
January 5, 2020 20:50
rerendering debug console on props change Error viewer console completed
Copy file spec completed Ngrok tunnel action spec updated
Ngrok module tests Testing tunnel error generation and capture scenarios
Code cleanup Renaming functions, test names, using shorthands wherever applicable
Updated icon for ngrok Status
Lock files updated after pull from master
|
@srinaath will remove the clients package-lock.json |
added 4 commits
January 6, 2020 16:06
tonyanziano
suggested changes
Jan 8, 2020
packages/app/client/.gitignore
Outdated
| 8000 | @@ -2,6 +2,7 @@ | ||
|
|
|||
| # dependencies | |||
| /node_modules | |||
| package-lock.json | |||
There was a problem hiding this comment.
Maybe we could instead add an entry to the root level .gitignore that ignores every lock file except for the root one?
/packages/**/package-lock.json
and then delete the sub-root-level lock files?
| logPath: string; | ||
| postmanCollectionPath: string; | ||
| tunnelStatus: TunnelStatus; | ||
| lastTunnelStatusCheckTS: string; |
There was a problem hiding this comment.
consider expanding to lastTunnelStatusCheckTimestamp for readability
| fs.copyFile(sourcePath, destinationPath, err => { | ||
| if (err) { | ||
| // eslint-disable-next-line no-console | ||
| console.error(`Error copying file: ${err}`); |
There was a problem hiding this comment.
maybe move this to the reject call so that it is surfaced to whatever context calls it:
reject(Error copying file: ${err});
| // OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION | ||
| // WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. | ||
|
|
||
| export default { |
There was a problem hiding this comment.
we prefer to use explicitly named exports in this project:
export const PostmanNgrokCollection = { ... };
Comment on lines
200
to
207
| protected async copyFile(sourcePath: string, destinationPath: string) { | ||
| try { | ||
| await copyFileAsync(sourcePath, destinationPath); | ||
| return true; | ||
| } catch (ex) { | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
Is there a reason this returns true and false? The only code that calls this in your PR calls it and doesn't worry about the return value because it is being called as the resolver of the executeCommand() action
packages/app/main/src/ngrok.ts
Outdated
| @@ -93,7 +99,30 @@ export class NgrokInstance { | |||
| return this.pendingConnection; | |||
| } | |||
| await this.getNgrokInspectUrl(options); | |||
| return this.runTunnel(options); | |||
| const tunnelInfo: { url; inspectUrl } = await this.runTunnel(options); | |||
| this.intervalForHealthCheck = setInterval(() => this.checkTunnelStatus.bind(this)(tunnelInfo.url), 60000); | |||
There was a problem hiding this comment.
A small optimization could be to pull the code that binds the function out of the setInterval() call so that .bind() is not called every time the function fires per interval:
const boundCheckTunnelStatus = this.checkTunnelStatus.bind(this);
this.intervalForHealthCheck = setInterval(() => boundCheckTunnelStatus(tunnelInfo.url), 60000);
added 7 commits
January 9, 2020 10:12
1. Added space after breaks 2. Seperating container and actual visual component 3. Removing nbsp 4. Adding package-locks to gitignore Signed-off-by: Srinaath Ravichandran <srravich@microsoft.com>
1.Using named exports 2. Returning errors from rejects 3. Removed dev artifacts Signed-off-by: Srinaath Ravichandran <srravich@microsoft.com>
Signed-off-by: Srinaath Ravichandran <srravich@microsoft.com>
1. Added space after breaks 2. Seperating container and actual visual component 3. Removing nbsp 4. Adding package-locks to gitignore Signed-off-by: Srinaath Ravichandran <srravich@microsoft.com> Pre feedback addressed 1.Using named exports 2. Returning errors from rejects 3. Removed dev artifacts Signed-off-by: Srinaath Ravichandran <srravich@microsoft.com> Made sure all pacjage locks are removed and spacing between cases Signed-off-by: Srinaath Ravichandran <srravich@microsoft.com> Untracked Package Locks Signed-off-by: Srinaath Ravichandran <srravich@microsoft.com> Untracked and deleted package locks Signed-off-by: Srinaath Ravichandran <srravich@microsoft.com>
Signed-off-by: Srinaath Ravichandran <srravich@microsoft.com>
Signed-off-by: Srinaath Ravichandran <srravich@microsoft.com>
Signed-off-by: Srinaath Ravichandran <srravich@microsoft.com>
tonyanziano
reviewed
Jan 10, 2020
packages/app/client/src/ui/editor/ngrokDebugger/ngrokDebuggerContainer.spec.tsx
Show resolved
Hide resolved
tonyanziano
reviewed
Jan 10, 2020
packages/app/client/src/ui/editor/ngrokDebugger/ngrokDebuggerContainer.tsx
Show resolved
Hide resolved
tonyanziano
reviewed
Jan 10, 2020
packages/app/client/src/ui/editor/ngrokDebugger/ngrokErrorHandler.tsx
Outdated
Show resolved
Hide resolved
tonyanziano
reviewed
Jan 10, 2020
packages/app/client/src/ui/editor/ngrokDebugger/ngrokErrorHandler.tsx
Outdated
Show resolved
Hide resolved
tonyanziano
reviewed
Jan 10, 2020
tonyanziano
reviewed
Jan 10, 2020
tonyanziano
suggested changes
Jan 10, 2020
srinaath
commented
Jan 10, 2020
Comments tag removed Typos fixed Signed-off-by: Srinaath Ravichandran <srravich@microsoft.com> Removed unused functions Signed-off-by: Srinaath Ravichandran <srravich@microsoft.com>
tonyanziano
reviewed
Jan 10, 2020
| return ( | ||
| <> | ||
| <legend> | ||
| Looks like you have hit your free tier limit on connections to your tunnel. These are few solutions. |
There was a problem hiding this comment.
missed this one last time: These are few solutions.
Maybe try something like:
(Consider | Try) the following solutions.
or
Below you will find several possible solutions.
tonyanziano
previously approved these changes
Jan 10, 2020
Signed-off-by: Srinaath Ravichandran <srravich@microsoft.com>
Thanks @tonyanziano |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ngrok Debug Console
Added console window to track tunnel errors
Pinned React version
More unit tests coverage in future PR's