Skip to content

Commit

Permalink
Improved error message when <module_name> is not received
Browse files Browse the repository at this point in the history
Previously this would lead to weird error message, now, if the input is likely a manifest, the error message will be super clear.
  • Loading branch information
maoueh committed Aug 28, 2023
1 parent 7fc7f5d commit d6f36cf
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 14 deletions.
9 changes: 5 additions & 4 deletions cmd/substreams/gui.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,11 @@ import (
"path/filepath"
"strings"

"github.com/streamingfast/substreams/manifest"

tea "github.com/charmbracelet/bubbletea"
"github.com/spf13/cobra"
"github.com/streamingfast/cli"
"github.com/streamingfast/substreams/client"
"github.com/streamingfast/substreams/manifest"
"github.com/streamingfast/substreams/tools"
"github.com/streamingfast/substreams/tui2"
"github.com/streamingfast/substreams/tui2/pages/request"
Expand Down Expand Up @@ -59,9 +58,11 @@ func runGui(cmd *cobra.Command, args []string) error {
manifestPath = args[0]
args = args[1:]
} else {
if cli.DirectoryExists(args[0]) || cli.FileExists(args[0]) || strings.Contains(args[0], ".") {
return fmt.Errorf("parameter entered likely a manifest file, don't forget to include a '<module_name>' in your command")
// Check common error where manifest is provided by module name is missing
if manifest.IsLikelyManifestInput(args[0]) {
return fmt.Errorf("missing <module_name> argument, check 'substreams run --help' for more information")
}

// At this point, we assume the user invoked `substreams run <module_name>` so we `resolveManifestFile` using the empty string since no argument has been passed.
manifestPath, err = resolveManifestFile("")
if err != nil {
Expand Down
7 changes: 6 additions & 1 deletion cmd/substreams/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package main
import (
"context"
"fmt"
"google.golang.org/grpc/metadata"
"io"
"strconv"
"strings"
Expand All @@ -17,6 +16,7 @@ import (
"github.com/streamingfast/substreams/tools/test"
"github.com/streamingfast/substreams/tui"
"go.uber.org/zap"
"google.golang.org/grpc/metadata"
)

func init() {
Expand Down Expand Up @@ -59,6 +59,11 @@ func runRun(cmd *cobra.Command, args []string) error {
var manifestPath, outputModule string
if len(args) == 1 {
outputModule = args[0]

// Check common error where manifest is provided by module name is missing
if manifest.IsLikelyManifestInput(outputModule) {
return fmt.Errorf("missing <module_name> argument, check 'substreams run --help' for more information")
}
} else {
manifestPath = args[0]
outputModule = args[1]
Expand Down
6 changes: 4 additions & 2 deletions docs/release-notes/change-log.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),

> [!IMPORTANT]
* The client and servers will both need to be upgraded at the same time for the new progress messages to be parsed:
- The new substreams servers will *NOT* send the old `modules` field as part of its `progress` message, only the new `running_jobs`, `modules_stats`, `stages`.
- The new substreams clients will *NOT* be able to decode the old progress information when connecting to older servers.
- The new Substreams servers will *NOT* send the old `modules` field as part of its `progress` message, only the new `running_jobs`, `modules_stats`, `stages`.
- The new Substreams clients will *NOT* be able to decode the old progress information when connecting to older servers.

* However, the actual data (and cursor) will work correctly between versions. Only incompatible progress information will be ignored.

Expand All @@ -29,6 +29,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),

* Bumped `substreams` and `substreams-ethereum` to latest in `substreams alpha init`.

* Improved error message when `<module_name>` is not received, previously this would lead to weird error message, now, if the input is likely a manifest, the error message will be super clear.

### Fixed

* Fixed compilation errors when tracking some contracts when using `substreams alpha init`.
Expand Down
37 changes: 30 additions & 7 deletions manifest/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
ipfs "github.com/ipfs/go-ipfs-api"
"github.com/jhump/protoreflect/desc"
"github.com/jhump/protoreflect/dynamic"
"github.com/streamingfast/cli"
"github.com/streamingfast/dstore"
pbsubstreams "github.com/streamingfast/substreams/pb/sf/substreams/v1"
"go.uber.org/zap"
Expand Down Expand Up @@ -121,7 +122,7 @@ func newReader(input string, workingDir string, opts ...Option) (*Reader, error)
}

func resolveInput(input string, workingDir string) (string, error) {
if isRemotePackage(input) {
if hasRemotePackagePrefix(input) {
return input, nil
}

Expand Down Expand Up @@ -207,16 +208,38 @@ func (r *Reader) read(workingDir string) (*pbsubstreams.Package, error) {
// IsRemotePackage determines if reader's input to read the manifest is a remote file accessible over
// HTTP/HTTPS, Google Cloud Storage, S3 or Azure Storage.
func (r *Reader) IsRemotePackage() bool {
return isRemotePackage(r.resolvedInput)
return hasRemotePackagePrefix(r.resolvedInput)
}

func isRemotePackage(in string) bool {
u, err := url.Parse(in)
if err != nil {
return false
func hasRemotePackagePrefix(in string) bool {
for _, prefix := range []string{"https://", "http://", "ipfs://", "gs://", "s3://", "az://"} {
if strings.HasPrefix(in, prefix) {
return true
}
}

return false
}

// IsLikelyManifestInput determines if the input is likely a manifest input, which is determined
// by checking:
// - If the input starts with remote prefix ("https://", "http://", "ipfs://", "gs://", "s3://", "az://")
// - If the input ends with `.yaml`
// - If the input is a directory (we check for path separator)
func IsLikelyManifestInput(in string) bool {
if hasRemotePackagePrefix(in) {
return true
}

if strings.HasSuffix(in, ".yaml") {
return true
}

if strings.Contains(in, string(os.PathSeparator)) {
return true
}

return u.Scheme == "http" || u.Scheme == "https" || u.Scheme == "gs" || u.Scheme == "s3" || u.Scheme == "az" || u.Scheme == "ipfs"
return cli.DirectoryExists(in) || cli.FileExists(in)
}

// IsLocalManifest determines if reader's input to read the manifest is a local manifest file, which is determined
Expand Down

0 comments on commit d6f36cf

Please sign in to comment.