Skip to content

Commit

Permalink
Fix InvalidRemoteErr panic (#2728)
Browse files Browse the repository at this point in the history
On an unknown remote ModuleIdentity may be nil. Use the fallback logic
to select the correct dependency and derive the related module identity.
Fixes #2725

A dial error for an invalid host will now correctly return the error
formatted with a prompt to check the registry:
```
Failure: dial tcp: lookup private.registry: no such host. Are you sure "private.registry" (derived from module name "private.registry/org/dependency") is a Buf Schema Registry?
```
  • Loading branch information
emcfarlane authored Jan 24, 2024
1 parent 2e1d627 commit 4b8e791
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 8 deletions.
13 changes: 9 additions & 4 deletions private/buf/cmd/buf/command/mod/modprune/modprune.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,13 @@ func run(
}
var dependencyModulePins []bufmoduleref.ModulePin
if len(requestReferences) > 0 {
var remote string
var (
remote string
moduleIdentityString string
)
if config.ModuleIdentity != nil && config.ModuleIdentity.Remote() != "" {
remote = config.ModuleIdentity.Remote()
moduleIdentityString = config.ModuleIdentity.IdentityString()
} else {
// At this point we know there's at least one dependency. If it's an unnamed module, select
// the right remote from the list of dependencies.
Expand All @@ -104,10 +108,11 @@ func run(
return fmt.Errorf(`File %q has invalid "deps" references`, existingConfigFilePath)
}
remote = selectedRef.Remote()
moduleIdentityString = selectedRef.IdentityString()
container.Logger().Debug(fmt.Sprintf(
`File %q does not specify the "name" field. Based on the dependency %q, it appears that you are using a BSR instance at %q. Did you mean to specify "name: %s/..." within %q?`,
existingConfigFilePath,
selectedRef.IdentityString(),
moduleIdentityString,
remote,
remote,
existingConfigFilePath,
Expand All @@ -125,8 +130,8 @@ func run(
}),
)
if err != nil {
if remote != bufconnect.DefaultRemote {
return bufcli.NewInvalidRemoteError(err, remote, config.ModuleIdentity.IdentityString())
if !connect.IsWireError(err) && remote != bufconnect.DefaultRemote {
return bufcli.NewInvalidRemoteError(err, remote, moduleIdentityString)
}
return err
}
Expand Down
13 changes: 9 additions & 4 deletions private/buf/cmd/buf/command/mod/modupdate/modupdate.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,13 @@ func getDependencies(
if len(moduleConfig.Build.DependencyModuleReferences) == 0 {
return nil, nil
}
var remote string
var (
remote string
moduleIdentityString string
)
if moduleConfig.ModuleIdentity != nil && moduleConfig.ModuleIdentity.Remote() != "" {
remote = moduleConfig.ModuleIdentity.Remote()
moduleIdentityString = moduleConfig.ModuleIdentity.IdentityString()
} else {
// At this point we know there's at least one dependency. If it's an unnamed module, select
// the right remote from the list of dependencies.
Expand All @@ -226,10 +230,11 @@ func getDependencies(
return nil, fmt.Errorf(`File %q has invalid "deps" references`, existingConfigFilePath)
}
remote = selectedRef.Remote()
moduleIdentityString = selectedRef.IdentityString()
container.Logger().Debug(fmt.Sprintf(
`File %q does not specify the "name" field. Based on the dependency %q, it appears that you are using a BSR instance at %q. Did you mean to specify "name: %s/..." within %q?`,
existingConfigFilePath,
selectedRef.IdentityString(),
moduleIdentityString,
remote,
remote,
existingConfigFilePath,
Expand Down Expand Up @@ -268,8 +273,8 @@ func getDependencies(
}),
)
if err != nil {
if remote != bufconnect.DefaultRemote {
return nil, bufcli.NewInvalidRemoteError(err, remote, moduleConfig.ModuleIdentity.IdentityString())
if !connect.IsWireError(err) && remote != bufconnect.DefaultRemote {
return nil, bufcli.NewInvalidRemoteError(err, remote, moduleIdentityString)
}
return nil, err
}
Expand Down

0 comments on commit 4b8e791

Please sign in to comment.