fail if an error log has occured #41

jhsinger-klotho · 2023-01-06T20:50:56Z

• Does any part of it require special attention?
• Does it relate to or fix any issue? closes #297

making compilation fail if an error is logged. Making consistent message at the end of the failure as well

case for logging an error

jordansinger@Jordans-MacBook-Pro ts-ms-lambda % klotho . --app ondemandddb -p aws -c test/klotho.yaml
██╗  ██╗██╗      ██████╗ ████████╗██╗  ██╗ ██████╗
██║ ██╔╝██║     ██╔═══██╗╚══██╔══╝██║  ██║██╔═══██╗
█████╔╝ ██║     ██║   ██║   ██║   ███████║██║   ██║
██╔═██╗ ██║     ██║   ██║   ██║   ██╔══██║██║   ██║
██║  ██╗███████╗╚██████╔╝   ██║   ██║  ██║╚██████╔╝
╚═╝  ╚═╝╚══════╝ ╚═════╝    ╚═╝   ╚═╝  ╚═╝ ╚═════╝

new update is available, please run klotho --update to get the latest version
Adding resource exec_unit:main
This is a test
| @klotho::persist {
|      id = 'users'
| }
| in users.js
| 8| const users = new Map();

This is a test
| @klotho::expose {
|      id = 'app'
|      target = 'public'
| }
| in index.js
| 36| app.listen(3000, async () => {
| 37|     console.log(`App listening at :3000`);
| 38| });

Found 2 route(s) for middleware 'router'                                                                                                                                                                                      unit: main
Adding resource gateway:app
Adding resource persist_kv:users
Adding resource topology:ondemandddb
Adding resource aws_template_data:ondemandddb
Adding resource infra_as_code:Pulumi (AWS)
Klotho compilation failed

case for an error being returned

jordansinger@Jordans-MacBook-Pro ts-ms-lambda % klotho . --app ondemandddb -p aws -c test/klotho.yaml
██╗  ██╗██╗      ██████╗ ████████╗██╗  ██╗ ██████╗
██║ ██╔╝██║     ██╔═══██╗╚══██╔══╝██║  ██║██╔═══██╗
█████╔╝ ██║     ██║   ██║   ██║   ███████║██║   ██║
██╔═██╗ ██║     ██║   ██║   ██║   ██╔══██║██║   ██║
██║  ██╗███████╗╚██████╔╝   ██║   ██║  ██║╚██████╔╝
╚═╝  ╚═╝╚══════╝ ╚═════╝    ╚═╝   ╚═╝  ╚═╝ ╚═════╝

new update is available, please run klotho --update to get the latest version
Adding resource exec_unit:main
Found 2 route(s) for middleware 'router'                                                                                                                                                                                      unit: main
Adding resource gateway:app
[err 0] error in plugin javascript/Express: error in plugin javascript/Express: 2 errors occurred:
	* This is also a test
	* This is also a test
Klotho compilation failed

Standard checks

Unit tests: Any special considerations? nope
Docs: Do we need to update any docs, internal or public? nope
Backwards compatibility: Will this break existing apps? If so, what would be the extra work required to keep them working? im not sure if we even log errors anywhere but if we do and people are deploying anyways, they wouldnt be able to now

jhsinger-klotho · 2023-01-06T20:52:25Z

cmd/klotho/main.go

@@ -83,11 +84,7 @@ func main() {

 	err := root.Execute()
 	if err != nil {
-		if cfg.internalDebug {


i was confused on why we have this. just reprints errors it looks like so i removed it. If theres a reason to have it ill add it back and have to change my implementation

The difference is in the format: %+v vs %v. The + gives a full stack trace, which is only relevant for us developers and not for users.

so do we want to have it as a cli flag then? because this could be exposed to the end user right?

jhsinger-klotho · 2023-01-06T20:52:52Z

cmd/klotho/main.go

 		analyticsClient.Error("klotho compiling failed")

 		cmd.SilenceErrors = true
 		cmd.SilenceUsage = true
-		return err
+		return errors.New("Failed run of klotho invocation")


because we always set SilenceErrors to true it shouldnt matter what we return, just that we return some error

gordon-klotho

The main parts look good, just comments on the extra changes included.

gordon-klotho · 2023-01-06T20:53:23Z

cmd/klotho/main.go

@@ -83,11 +84,7 @@ func main() {

 	err := root.Execute()
 	if err != nil {
-		if cfg.internalDebug {


The difference is in the format: %+v vs %v. The + gives a full stack trace, which is only relevant for us developers and not for users.

gordon-klotho · 2023-01-06T20:55:02Z

cmd/klotho/main.go

@@ -83,11 +84,7 @@ func main() {

 	err := root.Execute()
 	if err != nil {
-		if cfg.internalDebug {
-			zap.S().Errorf("%+v", err)
-		} else if !root.SilenceErrors {


Doesn't this regress the silence errors (which we set on line 270 so it doesn't double-print)?

nope, we still set that to make sure that we dont double print anything

yeah we actually will want this so that the non plugin errors surface, going to add it back

gordon-klotho · 2023-01-10T17:54:45Z

cmd/klotho/main.go

+		if err != nil {
+			errHandler.PrintErr(err)
+		} else {
+			err = errors.New("Failed run of klotho invocation")


I feel like this should go similar to the hadWarnings check (line 94)

that results in a double print as well though since we already log it. i think if we are treating error log lines and errors the same, having them go through the same flow is fine, it then treats zap errors as actual errors

we would also still print the output then if we did it that way which i thought this task is trying to prevent

True, we do want to prevent writing the output. I guess this error wouldn't get printed since we don't explicitly print it (like we do for err != nil) and also set SilenceErrors. 👍

github-actions · 2023-01-13T01:09:33Z

Package	Line Rate	Health
github.com/klothoplatform/klotho/pkg/analytics	2%	❌
github.com/klothoplatform/klotho/pkg/annotation	24%	❌
github.com/klothoplatform/klotho/pkg/core	20%	❌
github.com/klothoplatform/klotho/pkg/env_var	82%	✔
github.com/klothoplatform/klotho/pkg/exec_unit	45%	❌
github.com/klothoplatform/klotho/pkg/infra/kubernetes	58%	➖
github.com/klothoplatform/klotho/pkg/infra/kubernetes/helm	52%	➖
github.com/klothoplatform/klotho/pkg/input	63%	➖
github.com/klothoplatform/klotho/pkg/lang	37%	❌
github.com/klothoplatform/klotho/pkg/lang/dockerfile	0%	❌
github.com/klothoplatform/klotho/pkg/lang/golang	9%	❌
github.com/klothoplatform/klotho/pkg/lang/javascript	47%	❌
github.com/klothoplatform/klotho/pkg/lang/python	60%	➖
github.com/klothoplatform/klotho/pkg/lang/yaml	0%	❌
github.com/klothoplatform/klotho/pkg/logging	7%	❌
github.com/klothoplatform/klotho/pkg/multierr	95%	✔
github.com/klothoplatform/klotho/pkg/provider/aws	60%	➖
github.com/klothoplatform/klotho/pkg/runtime	75%	✔
github.com/klothoplatform/klotho/pkg/static_unit	32%	❌
github.com/klothoplatform/klotho/pkg/validation	73%	➖
Summary	42% (3592 / 8586)	❌

gordon-klotho · 2023-01-13T14:45:39Z

cmd/klotho/main.go

+		if err != nil {
+			errHandler.PrintErr(err)
+		} else {
+			err = errors.New("Failed run of klotho invocation")


True, we do want to prevent writing the output. I guess this error wouldn't get printed since we don't explicitly print it (like we do for err != nil) and also set SilenceErrors. 👍

jhsinger-klotho added 2 commits January 6, 2023 14:33

fail if an error log has occured

f44624e

consistent output

926f650

jhsinger-klotho commented Jan 6, 2023

View reviewed changes

gordon-klotho reviewed Jan 6, 2023

View reviewed changes

add back internal debug

fdc7029

gordon-klotho reviewed Jan 10, 2023

View reviewed changes

need to print errors if non plugin

73a21af

gordon-klotho approved these changes Jan 13, 2023

View reviewed changes

jhsinger-klotho merged commit a7a7511 into main Jan 13, 2023

jhsinger-klotho deleted the fail_on_error_log branch January 13, 2023 14:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fail if an error log has occured #41

fail if an error log has occured #41

jhsinger-klotho commented Jan 6, 2023

jhsinger-klotho Jan 6, 2023

gordon-klotho Jan 6, 2023

jhsinger-klotho Jan 6, 2023

jhsinger-klotho Jan 6, 2023

gordon-klotho left a comment

gordon-klotho Jan 6, 2023

gordon-klotho Jan 6, 2023

jhsinger-klotho Jan 6, 2023

jhsinger-klotho Jan 13, 2023

gordon-klotho Jan 10, 2023

jhsinger-klotho Jan 10, 2023

jhsinger-klotho Jan 13, 2023

gordon-klotho Jan 13, 2023

github-actions bot commented Jan 13, 2023

gordon-klotho Jan 13, 2023

fail if an error log has occured #41

fail if an error log has occured #41

Conversation

jhsinger-klotho commented Jan 6, 2023

Standard checks

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gordon-klotho left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 13, 2023

Choose a reason for hiding this comment