riok / Kreya

Kreya is a GUI client for gRPC and REST APIs with innovative features for environments, authorizations and more.
https://kreya.app
278 stars 5 forks source link

Failing to import `google/api` dependencies #237

Open mfridrikhson opened 3 weeks ago

mfridrikhson commented 3 weeks ago

Describe the bug Not sure if this is the expected behaviour or not, but as of 1.15.0 Kreya no longer implicitly loads the dependencies on Google's google/api common types. The error I'm getting:

google/api/annotations.proto: File not found.
v1/test.proto:7:1: Import "google/api/annotations.proto" was not found or had errors.

google/protobuf deps are getting loaded without any issues.

To Reproduce

  1. Create a proto file importing google/api/annotations.proto.
  2. Try to create a Kreya project importing the file.

Expected behavior Dependencies are being fetched without requiring to store them in the project directory.

Environment (if possible, copy the information from the error dialog or the About menu):

CommonGuy commented 3 weeks ago

We did some changes in Kreya 1.15 regarding importing proto files (we rely on the official protoc to do the parsing now). However, this should not have change anything for google/api imports, because those weren't implicitly imported by Kreya before.

Only types from google/protobuf are implicitly imported, which is the same behavior that protoc has. All others type have to be provided explicitely.

Are your google/api types located in the same place as your other protobuf files? If so, it could be that Kreya (mistakenly) picked them up in earlier version. Starting with Kreya 1.15, you may need to specifiy an additional import path (same as proto_path with protoc) so that Kreya finds the additional types.

mfridrikhson commented 3 weeks ago

@CommonGuy We are using Buf for protobuf development, so actually we've never been downloading the google dependencies to the project directory since one of the features of Buf is that it's able to fetch the dependencies on compilation.

In that case I'm clueless how Kreya was resolving them πŸ€”

CommonGuy commented 2 weeks ago

Correction, the types from google/api/annotations.proto and google/api/http.proto were implicitely loaded in Kreya 1.14 (due to the library used). Switching to the official protoc implementation lost us this feature. This will be fixed in the next release.

sneaky-peeky commented 2 weeks ago

@CommonGuy is there any way to install older version of kreya? like 1.14.1 for example

CommonGuy commented 1 week ago

@sneaky-peeky We currently do not provide a way to download older versions.

However, we have released a fix for this on our beta release channel. If you want to try it, see https://kreya.app/docs/faq/#how-can-i-use-kreya-beta-versions

sgielen commented 1 week ago

@CommonGuy I had the same issue in Kreya 1.15.0, just upgraded to 1.15.1 beta and now the issue changed to google.api.http being loaded twice:

google/api/http.proto:33:21: "google.api.Http.rules" is already defined in file "node_modules/protobufjs/google/api/http.proto".
google/api/http.proto:29:9: "google.api.Http" is already defined in file "node_modules/protobufjs/google/api/http.proto".
google/api/http.proto: "google.api.HttpRule.pattern" is already defined in file "node_modules/protobufjs/google/api/http.proto".
google/api/http.proto:321:10: "google.api.HttpRule.selector" is already defined in file "node_modules/protobufjs/google/api/http.proto".
[....etc....]

In my repo, the protofiles are in pkg/proto but there are also some proto files in pkg/proto/node_modules, and there is no way to load pkg/proto/*.proto without also loading pkg/proto/**/*.proto. So all the proto files in node_modules are loaded as well, which includes a second copy of google/api/http.proto. In Kreya 1.14, this wasn't an issue.

ni507 commented 1 week ago

@sgielen I cannot reproduce this. Is the http.proto only in pkg/proto/node_modules/... or also a second time in pkg/proto ? Or is it maybe twice in pkg/proto/node_modules/... ?

sgielen commented 1 week ago

@ni507 It is only once in node_modules, but I just noticed that only http.proto does not cause the issue, but having also annotations.proto in the same directory does cause the issue. Perhaps because http.proto itself is parsed, and then annotations.proto also imports http.proto and this causes the double definitions issue?

In case it matters, I've attached my exact two copies. Renamed to .txt, so that upload to github is allowed.

annotations.proto.txt http.proto.txt

google/api/http.proto:33:21: "google.api.Http.rules" is already defined in file "node_modules/protobufjs/google/api/http.proto".
google/api/http.proto:29:9: "google.api.Http" is already defined in file "node_modules/protobufjs/google/api/http.proto".
google/api/http.proto: "google.api.HttpRule.pattern" is already defined in file "node_modules/protobufjs/google/api/http.proto".
[...]
node_modules/protobufjs/google/api/annotations.proto:5:1: Import "google/api/http.proto" was not found or had errors.
node_modules/protobufjs/google/api/annotations.proto:10:5: "google.api.HttpRule" seems to be defined in "node_modules/protobufjs/google/api/http.proto", which is not imported by "node_modules/protobufjs/google/api/annotations.proto".  To use it here, please add the necessary import.
sgielen commented 1 week ago

@ni507 just checking - can you reproduce this on your end with this information, or is there something I could investigate further to help reproduce this?

ni507 commented 1 week ago

@sgielen I was able to reproduce it, but only in a way, as it did not work in version 1.14 either.

I created a folder with protos and this structure:

protos
β”‚   some.proto 
β”‚   ....
└───node_modules
β”‚   β”‚
β”‚   └───google/api
β”‚       β”‚   http.proto
β”‚       β”‚   annotations.proto

Then I get your error because I used import "google/api/http.proto"; in the some.proto file. But I also tried this in Kreya 1.14, which also resulted in an error. To fix this in the current beta version, I added an import path in the importer settings to the node_modules folder like this {insert here your path}/protos/node_modules. Then the import works.

Can you try adding an import path to your importer as well? image

I'm wondering why your setup worked on version 1.14. Could something be different in my example?

sgielen commented 1 week ago

@sgielen I was able to reproduce it, but only in a way, as it did not work in version 1.14 either.

That is interesting.

I did some further investigating by removing files from my node_modules until I had the minimum set of files that caused the error. Lo and behold, it's just those two files I sent earlier: annotations.proto and http.proto, even outside of any node_modules. So for me, just having those two files reproduces the issue on 1.15.1beta1 in a way that doesn't seem to happen for you, is that correct?

I have a video here to show how I reproduce this with just those two files in my proto path:

https://github.com/riok/Kreya/assets/328769/62a62abb-5525-4a93-aa83-d25bafbbab9d In case that doesn't work, here's the video in a .zip file: reproduce.zip And the exact two proto files in a .zip: proto.zip

Does Kreya use anything from the host (protoc/proto libraries) in a way that could change our outcomes? (This might also be the reason that creating a project takes a relatively long time, or is that normal?)

I'm not 100% certain my earlier installed Kreya version was 1.14, but I am certain I didn't change anything in node_modules between going from that version to 1.15. If you want, I can try to find a 1.14 download and perform the same steps with it on my machine?

hoyau commented 1 week ago

@sgielen I've verified it with a similar setup.

When i have a subfolder with the 2 files annotations.proto and http.proto it will not generate an error during the import in v1.14, while in v1.15.1-beta.1 it does.

We will look into this with high priority.
It seems the only workaround right now is to explicitly remove these 2 files from your project, since they are imported per default in v1.15.1-beta.1.

sgielen commented 1 week ago

@hoyau thank you for this! I can’t remove them unfortunately, as they are needed during the build. But, for now, as a workaround, I just copy the proto files elsewhere and import from there until it’s fixed in a newer version. Thanks a lot for the fast responses!