chore(cli): send telemetry on errors #829
Conversation
DanielMSchmidt
force-pushed
the
telemetry-improvements
branch
from
f67d2b0 to
1ecd59c
Compare
DanielMSchmidt
force-pushed
the
telemetry-improvements
branch
from
1ecd59c to
77015f5
Compare
packages/cdktf-cli/bin/cmds/init.ts
Outdated
| @@ -386,17 +393,17 @@ async function fetchRemoteTemplate(templateUrl: string): Promise<Template> { | |||
| console.error( | |||
| chalkColour`Please supply a valid url (including the protocol) or use one of the built-in templates.` | |||
| ); | |||
| process.exit(1); | |||
| return await process.exit(1); | |||
There was a problem hiding this comment.
Just read a bit on process.exit in the docs and it seems that we should refrain from using such calls. I have used them myself extensively in the past, but they'll make our error reporting unreliable as errors may not always have been sent completely by the time the process exits.
On another note: This is also why we don't need to await the exit and return is also not needed 😅
Fixes #828 Date: Tue Jul 20 18:15:43 2021 +0200
DanielMSchmidt
force-pushed
the
telemetry-improvements
branch
from
77015f5 to
3ffe2ab
Compare
DanielMSchmidt
force-pushed
the
telemetry-improvements
branch
from
3ffe2ab to
284589b
Compare
| @@ -397,8 +398,10 @@ export const determineGoModuleName = async (dir: string): Promise<string> => { | |||
| const childdir = path.relative(currentDir, dir).replace(/\\/g, "/"); // replace '\' with '/' for windows paths | |||
| return childdir.length > 0 ? `${match[1]}/${childdir}` : match[1]; | |||
| } | |||
| throw new Error( | |||
| `Could not determine the root Go module name. Found ${file} but failed to regex match the module name directive` | |||
| throw await Errors.Internal( | |||
There was a problem hiding this comment.
The await has no effect as report() is not awaited in reportPrefixedErrors
| @@ -407,7 +410,9 @@ export const determineGoModuleName = async (dir: string): Promise<string> => { | |||
| currentDir = path.dirname(currentDir); | |||
| } while (currentDir !== previousDir); | |||
|
|
|||
| throw new Error( | |||
| `Could not determine the root Go module name. No go.mod found in ${dir} and any parent directories` | |||
| throw await Errors.Usage( | |||
packages/cdktf-cli/lib/errors.ts
Outdated
| } | ||
|
|
||
| function reportPrefixedError(type: string) { | ||
| return (command: string, message: string, context: Record<string, any>) => { |
There was a problem hiding this comment.
Maybe make the context optional? (Sometimes an empty object is passed)
DanielMSchmidt
force-pushed
the
telemetry-improvements
branch
from
a44f2cc to
1b2f0db
Compare
DanielMSchmidt
force-pushed
the
telemetry-improvements
branch
from
7aba0f3 to
bdc22b6
Compare
|
I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you've found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further. |
This PR helps us track errors to understand the problems of our user-base better.
I added the Errors object in an adjecent effort of being deliberate about Error types