From 7aef56298405055242918e70d04d601f2ed9a1c9 Mon Sep 17 00:00:00 2001
From: David Sharnoff <github@dave.sharnoff.org>
Date: Sat, 14 Jan 2023 20:15:53 -0800
Subject: [PATCH] docs: some more minor clarifications and examples (#52)

* more doc adjustments

* a few more doc changes
---
 doc.go   | 39 +++++++++++++++++++++++++++++++++++----
 types.go |  6 ++++++
 2 files changed, 41 insertions(+), 4 deletions(-)

diff --git a/doc.go b/doc.go
index 6e553c1..d32fe62 100644
--- a/doc.go
+++ b/doc.go
@@ -175,8 +175,8 @@ provider chain returns from that point with the TerminalError as a return
 value.  Since all return values must be consumed by a middleware provider or
 the bound invoke function,
 fallible injectors must come downstream from a middleware handler that
-takes TerminalError as a returned value if the invoke function does not
-return error.  If a fallible injector returns
+takes error as a returned value if the invoke function (function that runs
+a bound injection chain) does not return error.  If a fallible injector returns
 nil for the TerminalError, the other output values are made available
 for downstream handlers to consume.  The other output values are not
 considered return values and are not available to be consumed by upstream
@@ -253,6 +253,29 @@ an upstream wrapper function or by the init function (if using Bind()).
 
 	func(input value(s)) return values(s)
 
+Wrap functions that return error should take error as a returned value so that
+they do not mask a downstream error.  Wrap functions should not return TerminalError
+because they internally control if the downstream chain is called.
+
+	func GoodExample(inner func() error) error {
+		if err := DoSomething(); err != nil {
+			// skip remainder of injection chain
+			return err
+		}
+		err := inner()
+		return err
+	}
+
+	func BadExampleMasksDownstreamError(inner func()) error {
+		if err := DoSomething(); err != nil {
+			// skip remainder of injection chain
+			return err
+		}
+		inner()
+		// nil is returned even if a downsteam injector returns error
+		return nil
+	}
+
 Literal values
 
 Literal values are values in the provider chain that are not functions.
@@ -378,6 +401,9 @@ error is generally very good, but it does not contain the full debugging
 output.
 
 The full debugging output can be obtained with the DetailedError function.
+If the detailed error shows that nject has a bug, note that part of the debug
+output includes a regression test that can be turned into an nject issue.
+Remove the comments to hide the original type names.
 
 	err := nject.Run("some chain", some, injectors)
 	if err != nil {
@@ -391,7 +417,7 @@ Reorder
 
 The Reorder() decorator allows injection chains to be fully or partially reordered.
 Reorder is currently limited to a single pass and does not know which injectors are
-ultimately going to be included in the final chain.  It is likely that if you mark
+ultimately going to be included in the final chain. It is likely that if you mark
 your entire chain with Reorder, you'll have unexpected results.  On the other hand,
 Reorder provides safe and easy way to solve some common problems.
 
@@ -404,7 +430,7 @@ For example: providing optional options to an injected dependency.
 
 	func DefaultThingOptions() []ThingOption {
 		return []ThingOption{
-			StanardThingOption
+			StanardThingOption,
 		}
 	}
 
@@ -465,5 +491,10 @@ For services, something like t.Cleanup can easily be built:
 		return thing
 	}
 
+Alternatively, any wrapper function can do it's own cleanup in a defer that it
+defines.  Wrapper functions have a small runtime performance penalty, so if you
+have more than a couple of providers that need cleanup, it makes sense to include
+something like CleaningService.
+
 */
 package nject
diff --git a/types.go b/types.go
index a12665b..ffe6c74 100644
--- a/types.go
+++ b/types.go
@@ -19,6 +19,12 @@ import (
 // and (like other return values) it must be consumed by an upstream handler
 // or the invoke function.
 //
+// Functions that return TerminalError do not count as having no outputs and
+// thus they are not treated as specially required.  The when TerminalError is
+// returned, if it is nil, then it provides a nil error to the rest of the
+// downard chain. If it returns non-nil, then it provides a non-nil error as a
+// returned value to the upwards chain.
+//
 // Note: wrapper functions should not return TerminalError because such
 // a return value would not be automatically converted into a regular error.
 type TerminalError interface {