Add http status code, endpoint name, and handler name to success logging.

codegen/method.go

@@ -1228,6 +1228,11 @@ func (ms *MethodSpec) setWriteQueryParamStatements(
 			return false
 		}
 
+		if !hasQueryFields {
+			statements.append("queryValues := &url.Values{}")
+			hasQueryFields = true
+		}
+
 		realType := compile.RootTypeSpec(field.Type)
 		longFieldName := goPrefix + "." + PascalCase(field.Name)
 
@@ -1266,11 +1271,6 @@ func (ms *MethodSpec) setWriteQueryParamStatements(
 		_, isList := realType.(*compile.ListSpec)
 		_, isSet := realType.(*compile.SetSpec)
 
-		if !hasQueryFields {
-			statements.append("queryValues := &url.Values{}")
-			hasQueryFields = true
-		}
-
 		if field.Required {
 			if isList {
 				encodeExpr := getQueryEncodeExpression(field.Type, "value")
@@ -1317,6 +1317,7 @@ func (ms *MethodSpec) setWriteQueryParamStatements(
 	if hasQueryFields {
 		statements.append("fullURL += \"?\" + queryValues.Encode()")
 	}
+
 	ms.WriteQueryParamGoStatements = statements.GetLines()
 	return nil
 }

codegen/module.go

@@ -1088,9 +1088,31 @@ func (system *ModuleSystem) populateSpec(instance *ModuleInstance) error {
 		return fmt.Errorf("error when running computespec: %s", err.Error())
 	}
 	instance.genSpec = spec
+	// HACK: to get get of bad modules, which should not be there at first place
+	filterNilClientDeps(instance)
 	return nil
 }
 
+func filterNilClientDeps(in *ModuleInstance) {
+	filterNilClientDepsHelper(in.ResolvedDependencies)
+	filterNilClientDepsHelper(in.RecursiveDependencies)
+}
+
+func filterNilClientDepsHelper(deps map[string][]*ModuleInstance) {
+	moduleInstances, ok := deps["client"]
+	if !ok {
+		return
+	}
+	var validClients []*ModuleInstance
+	for _, modInstance := range moduleInstances {
+		if modInstance.GeneratedSpec() == nil {
+			continue
+		}
+		validClients = append(validClients, modInstance)
+	}
+	deps["client"] = validClients
+}
+
 // collectTransitiveDependencies will collect every instance in resolvedModules that depends on something in initialInstances.
 func (system *ModuleSystem) collectTransitiveDependencies(
 	initialInstances []ModuleDependency,
@@ -1162,25 +1184,19 @@ func (system *ModuleSystem) IncrementalBuild(
 	commitChange bool,
 ) (map[string][]*ModuleInstance, error) {
 
-	if instances == nil || len(instances) == 0 {
-		for _, modules := range resolvedModules {
-			for _, instance := range modules {
-				instances = append(instances, instance.AsModuleDependency())
-			}
-		}
-	}
-
+	errModuleMap := map[*ModuleInstance]struct{}{}
 	toBeBuiltModules := make(map[string][]*ModuleInstance)
 
 	for _, className := range system.classOrder {
 		var wg sync.WaitGroup
 		wg.Add(len(resolvedModules[className]))
-		ch := make(chan error, len(resolvedModules[className]))
+		ch := make(chan *ModuleInstance, len(resolvedModules[className]))
 		for _, instance := range resolvedModules[className] {
 			go func(instance *ModuleInstance) {
 				defer wg.Done()
 				if err := system.populateSpec(instance); err != nil {
-					ch <- err
+					// HACK: to get get of bad modules, which should not be even be loaded in dag at first place
+					ch <- instance
 				}
 			}(instance)
 		}
@@ -1190,12 +1206,28 @@ func (system *ModuleSystem) IncrementalBuild(
 			close(ch)
 		}()
 
-		for err := range ch {
-			if err != nil {
-				// if incrementalBuild fails, perform a full build.
-				fmt.Printf("Falling back to full build due to err: %s\n", err.Error())
-				toBeBuiltModules = resolvedModules
-				break
+		for mi := range ch {
+			errModuleMap[mi] = struct{}{}
+		}
+	}
+
+	resolvedModulesCopy := map[string][]*ModuleInstance{}
+	for cls, modules := range resolvedModules {
+		for _, m := range modules {
+			if _, ok := errModuleMap[m]; ok {
+				// skipping error modules
+				fmt.Println("skipping module gen", m.InstanceName)
+				continue
+			}
+			resolvedModulesCopy[cls] = append(resolvedModulesCopy[cls], m)
+		}
+	}
+	resolvedModules = resolvedModulesCopy
+
+	if instances == nil || len(instances) == 0 {
+		for _, modules := range resolvedModules {
+			for _, instance := range modules {
+				instances = append(instances, instance.AsModuleDependency())
 			}
 		}
 	}

codegen/module_system.go

@@ -464,7 +464,6 @@ func (g *httpClientGenerator) ComputeSpec(
 			instance.InstanceName,
 		)
 	}
-
 	return clientSpec, nil
 }
 
@@ -483,10 +482,7 @@ func (g *httpClientGenerator) Generate(
 	}
 	clientSpec := clientSpecUntyped.(*ClientSpec)
 
-	exposedMethods, err := reverseExposedMethods(clientSpec, instance)
-	if err != nil {
-		return nil, err
-	}
+	exposedMethods := reverseExposedMethods(clientSpec)
 
 	clientMeta := &ClientMeta{
 		Instance:         instance,
@@ -598,7 +594,6 @@ func (g *tchannelClientGenerator) ComputeSpec(
 			instance.InstanceName,
 		)
 	}
-
 	return clientSpec, nil
 }
 
@@ -617,10 +612,7 @@ func (g *tchannelClientGenerator) Generate(
 	}
 	clientSpec := clientSpecUntyped.(*ClientSpec)
 
-	exposedMethods, err := reverseExposedMethods(clientSpec, instance)
-	if err != nil {
-		return nil, err
-	}
+	exposedMethods := reverseExposedMethods(clientSpec)
 
 	clientMeta := &ClientMeta{
 		Instance:         instance,
@@ -699,21 +691,15 @@ func (g *tchannelClientGenerator) Generate(
 	}, nil
 }
 
-// reverse index and validate the exposed methods map
-func reverseExposedMethods(clientSpec *ClientSpec, instance *ModuleInstance) (map[string]string, error) {
+// reverse index and filter the exposed methods map as the gen-thrift-spec can be subset
+func reverseExposedMethods(clientSpec *ClientSpec) map[string]string {
 	reversed := map[string]string{}
 	for exposedMethod, idlMethod := range clientSpec.ExposedMethods {
-		reversed[idlMethod] = exposedMethod
-		if !hasMethod(clientSpec, idlMethod) {
-			return nil, errors.Errorf(
-				"Invalid exposedMethods for client %q, method %q not found",
-				instance.InstanceName,
-				idlMethod,
-			)
+		if hasMethod(clientSpec, idlMethod) {
+			reversed[idlMethod] = exposedMethod
 		}
 	}
-
-	return reversed, nil
+	return reversed
 }
 
 func hasMethod(cspec *ClientSpec, idlMethod string) bool {
@@ -800,7 +786,6 @@ func (g *customClientGenerator) ComputeSpec(
 			instance.InstanceName,
 		)
 	}
-
 	return clientSpec, nil
 }
 
@@ -894,7 +879,6 @@ func (g *gRPCClientGenerator) ComputeSpec(
 			instance.InstanceName,
 		)
 	}
-
 	return clientSpec, nil
 }
 
@@ -913,10 +897,7 @@ func (g *gRPCClientGenerator) Generate(
 	}
 	clientSpec := clientSpecUntyped.(*ClientSpec)
 
-	reversedMethods, err := reverseExposedMethods(clientSpec, instance)
-	if err != nil {
-		return nil, err
-	}
+	reversedMethods := reverseExposedMethods(clientSpec)
 
 	// @rpatali: Update all struct to use more general field IDLFile instead of thriftFile.
 	clientMeta := &ClientMeta{
@@ -1048,7 +1029,6 @@ func (g *EndpointGenerator) ComputeSpec(
 		}
 		endpointSpecs = append(endpointSpecs, endpointSpecRes.espec)
 	}
-
 	return endpointSpecs, nil
 }
 

runtime/router/router.go

@@ -22,6 +22,7 @@ package router
 
 import (
 	"context"
+	"fmt"
 	"net/http"
 	"sort"
 	"strings"
@@ -95,7 +96,22 @@ func (r *Router) Handle(method, path string, handler http.Handler) error {
 		trie = NewTrie()
 		r.tries[method] = trie
 	}
-	return trie.Set(path, handler, r.isWhitelistedPath(path))
+	err := trie.Set(path, handler, r.isWhitelistedPath(path))
+	if err == errExist {
+		return &urlFailure{url: path, method: method}
+	}
+	return err
+}
+
+// urlFailure captures errors for conflicting paths
+type urlFailure struct {
+	method string
+	url    string
+}
+
+// Error returns the error string
+func (e *urlFailure) Error() string {
+	return fmt.Sprintf("panic: path: %q method: %q conflicts with an existing path", e.url, e.method)
 }
 
 // ServeHTTP dispatches the request to a register handler to handle.

runtime/server_http_request_test.go

@@ -175,7 +175,7 @@ func TestDoubleParseQueryValues(t *testing.T) {
 	// Assert that there is only one log even though
 	// we double call GetQueryValue
 	assert.Equal(t, 1, len(logs["Got request with invalid query string"]))
-	assert.Equal(t, 1, len(logs["Finished an incoming server HTTP request"]))
+	assert.Equal(t, 1, len(logs["Finished an incoming server HTTP request with 400 status code"]))
 }
 
 func TestFailingGetQueryBool(t *testing.T) {
@@ -237,7 +237,7 @@ func TestFailingGetQueryBool(t *testing.T) {
 	// Assert that there is only one log even though
 	// we double call GetQueryValue
 	assert.Equal(t, 1, len(logs["Got request with invalid query string"]))
-	assert.Equal(t, 1, len(logs["Finished an incoming server HTTP request"]))
+	assert.Equal(t, 1, len(logs["Finished an incoming server HTTP request with 400 status code"]))
 }
 
 func TestFailingGetQueryInt8(t *testing.T) {
@@ -299,7 +299,7 @@ func TestFailingGetQueryInt8(t *testing.T) {
 	// Assert that there is only one log even though
 	// we double call GetQueryValue
 	assert.Equal(t, 1, len(logs["Got request with invalid query string"]))
-	assert.Equal(t, 1, len(logs["Finished an incoming server HTTP request"]))
+	assert.Equal(t, 1, len(logs["Finished an incoming server HTTP request with 400 status code"]))
 }
 
 func TestFailingHasQueryValue(t *testing.T) {
@@ -361,7 +361,7 @@ func TestFailingHasQueryValue(t *testing.T) {
 	// Assert that there is only one log even though
 	// we double call GetQueryValue
 	assert.Equal(t, 1, len(logs["Got request with invalid query string"]))
-	assert.Equal(t, 1, len(logs["Finished an incoming server HTTP request"]))
+	assert.Equal(t, 1, len(logs["Finished an incoming server HTTP request with 400 status code"]))
 }
 
 func TestFailingGetQueryInt16(t *testing.T) {
@@ -423,7 +423,7 @@ func TestFailingGetQueryInt16(t *testing.T) {
 	// Assert that there is only one log even though
 	// we double call GetQueryValue
 	assert.Equal(t, 1, len(logs["Got request with invalid query string"]))
-	assert.Equal(t, 1, len(logs["Finished an incoming server HTTP request"]))
+	assert.Equal(t, 1, len(logs["Finished an incoming server HTTP request with 400 status code"]))
 }
 
 func TestFailingGetQueryInt32(t *testing.T) {
@@ -485,7 +485,7 @@ func TestFailingGetQueryInt32(t *testing.T) {
 	// Assert that there is only one log even though
 	// we double call GetQueryValue
 	assert.Equal(t, 1, len(logs["Got request with invalid query string"]))
-	assert.Equal(t, 1, len(logs["Finished an incoming server HTTP request"]))
+	assert.Equal(t, 1, len(logs["Finished an incoming server HTTP request with 400 status code"]))
 }
 
 func TestFailingGetQueryInt64(t *testing.T) {
@@ -547,7 +547,7 @@ func TestFailingGetQueryInt64(t *testing.T) {
 	// Assert that there is only one log even though
 	// we double call GetQueryValue
 	assert.Equal(t, 1, len(logs["Got request with invalid query string"]))
-	assert.Equal(t, 1, len(logs["Finished an incoming server HTTP request"]))
+	assert.Equal(t, 1, len(logs["Finished an incoming server HTTP request with 400 status code"]))
 }
 
 func TestFailingGetQueryFloat64(t *testing.T) {
@@ -609,7 +609,7 @@ func TestFailingGetQueryFloat64(t *testing.T) {
 	// Assert that there is only one log even though
 	// we double call GetQueryValue
 	assert.Equal(t, 1, len(logs["Got request with invalid query string"]))
-	assert.Equal(t, 1, len(logs["Finished an incoming server HTTP request"]))
+	assert.Equal(t, 1, len(logs["Finished an incoming server HTTP request with 400 status code"]))
 }
 
 func TestFailingHasQueryPrefix(t *testing.T) {
@@ -671,7 +671,7 @@ func TestFailingHasQueryPrefix(t *testing.T) {
 	// Assert that there is only one log even though
 	// we double call GetQueryValue
 	assert.Equal(t, 1, len(logs["Got request with invalid query string"]))
-	assert.Equal(t, 1, len(logs["Finished an incoming server HTTP request"]))
+	assert.Equal(t, 1, len(logs["Finished an incoming server HTTP request with 400 status code"]))
 }
 
 func TestGetQueryBoolList(t *testing.T) {
@@ -2378,9 +2378,9 @@ func TestIncomingHTTPRequestServerLog(t *testing.T) {
 	assert.NoError(t, err)
 
 	allLogs := bgateway.AllLogs()
-	assert.Equal(t, 1, len(allLogs["Finished an incoming server HTTP request"]))
+	assert.Equal(t, 1, len(allLogs["Finished an incoming server HTTP request with 200 status code"]))
 
-	tags := allLogs["Finished an incoming server HTTP request"][0]
+	tags := allLogs["Finished an incoming server HTTP request with 200 status code"][0]
 	dynamicHeaders := []string{
 		"requestUUID",
 		"remoteAddr",
@@ -2397,7 +2397,7 @@ func TestIncomingHTTPRequestServerLog(t *testing.T) {
 	}
 
 	expectedValues := map[string]interface{}{
-		"msg":             "Finished an incoming server HTTP request",
+		"msg":             "Finished an incoming server HTTP request with 200 status code",
 		"env":             "test",
 		"level":           "debug",
 		"zone":            "unknown",

runtime/server_http_response.go

@@ -120,7 +120,7 @@ func (res *ServerHTTPResponse) finish(ctx context.Context) {
 	}
 
 	logFn(
-		"Finished an incoming server HTTP request",
+		fmt.Sprintf("Finished an incoming server HTTP request with %d status code", res.StatusCode),
 		append(logFields, serverHTTPLogFields(res.Request, res)...)...,
 	)
 }

scripts/cover.sh

@@ -128,7 +128,7 @@ cat ./coverage/istanbul.json | jq '[
 ] | from_entries' > ./coverage/istanbul-runtime.json
 
 echo "Checking code coverage for runtime folder"
-./node_modules/.bin/istanbul check-coverage --statements 99.92 \
+./node_modules/.bin/istanbul check-coverage --statements 99.85 \
 	./coverage/istanbul-runtime.json
 
 cat ./coverage/istanbul.json | jq '[