grpc / grpc-dart

The Dart language implementation of gRPC.
https://pub.dev/packages/grpc
Apache License 2.0
861 stars 271 forks source link

Any cannot be imported #589

Closed TamasBarta closed 2 years ago

TamasBarta commented 2 years ago

After depending on grpc and importing it as well, Any is still unavailable.

Version of the grpc-dart packages and protobuf (just in case) used:

  grpc:
    dependency: "direct main"
    description:
      name: grpc
      url: "https://pub.dartlang.org"
    source: hosted
    version: "3.1.0"
  protobuf:
    dependency: "direct main"
    description:
      name: protobuf
      url: "https://pub.dartlang.org"
    source: hosted
    version: "2.1.0"

Repro steps

It's not runtime behaviour, but to put in this format:

  1. Depend on grpc package (and run dart pub get)
  2. Try to import Any
  3. dartls cannot find it.
  4. Import package:protobuf/protobuf.dart and package:grpc/grpc.dart anyway
  5. Still cannot find Any
  6. Try to compile to see if it's a problem with dartls, no luck

Expected result: Any should be available after depending on grpc in pubspec.yaml a

Actual result: Any nowhere to be found.

Details

I'm trying to implement some sort of error handling, and found that GrpcError's details are all Any. This is already pretty cumbersome, but I could work around that, and that's understandable why it's like that. I can just use the solution in #461 to get an instance of the concrete message type. The problem is that Any can not be imported, and I didn't even find basically anywhere on the internet, most notably the grpc package's API reference documentation. I believe this is an error.

Right now I could make this work by manually casting it to dynamic which is unnecessarily javascriptish (not to hate on it, but I deliberately chose Dart instead), and I'm sure it's not the intended way to do error handling.

I tried import "google/protobuf/any.proto"; in our proto files to at least have any sort of Any, but type checks don't work on it, so I cannot use it to call its methods, not useful for error handling. (Also the proto project itself doesn't depend on it, so wouldn't be a good solution anyway)

mraleph commented 2 years ago

Currently you have two options.

The first option is to simply depend on package:grpc/src/generated/google/protobuf/any.pb.dart.

The other option is to take https://github.com/protocolbuffers/protobuf/blob/main/src/google/protobuf/any.proto put it into google/protobuf/any.proto next to your protobuf definitions and then use result of that in your project.

TamasBarta commented 1 year ago

The first option works, but results in a warning by the linter, the second option simply doesn't work. (I mentioned it in the first post, but I forgot to mention that I use buf so that takes care of generating classes for imports too)

While the "I can't import it" is technically solved by the first option from your suggestions (resulting in a warning), and I don't mind this issue being closed, but error handling with rich error models is broken in Dart. I don't know where you want to track it, but I kindly ask you to consider coming up with a different solution.

The simplest could be to reexport (I don't know the proper Dart term, sorry) the generated classes that are expected by the authors to be used by the user, so error handling is not a hack with importing from src and warning suppressions.

mraleph commented 1 year ago

the second option simply doesn't work.

It does work - that's how package:grpc/src/generated/google/protobuf/any.pb.dart is generated in the first place.

The simplest could be to reexport

I think the only reason why these classes are not exported from protobuf package itself is because you don't want generated code to get out of sync with your .proto files, e.g. you might edit your copy of any.proto file and expect any.pb.dart to change. (Not that it is a likely thing to happen in most cases though). The same concern applies to grpc package itself.

I have filed https://github.com/google/protobuf.dart/issues/780

TamasBarta commented 1 year ago

the second option simply doesn't work.

It does work - that's how package:grpc/src/generated/google/protobuf/any.pb.dart is generated in the first place.

What I meant by that doesn't work is that I tried it, it generated an identical Any implementation, but (myDetailThatIsAny is Any) == false (where the variable comes from grpc, and the type is generated by me), and I have to check for that as far as I understood the switch case here, because not everything is an Any in that list. (I got the impression that some details are Any, and some are unpacked into some well know types.)

The simplest could be to reexport

I think the only reason why these classes are not exported from protobuf package itself is because you don't want generated code to get out of sync with your .proto files, e.g. you might edit your copy of any.proto file and expect any.pb.dart to change. (Not that it is a likely thing to happen in most cases though). The same concern applies to grpc package itself.

I don't know if any.proto is supposed to be changed by the user, but my understanding is that it is namespeced under Google, so it shouldn't. I for example didn't copy it, but "vendored" it with buf. But that's just philosophy, users may actually alter their any.proto in the wild, I don't know that, so I trust you with that being a use case.

I have filed google/protobuf.dart#780

Thank you, I appreciate your attention and work on this topic. I'll subscribe there.

mraleph commented 1 year ago

Ah, I see what you mean. Indeed is and as would not work if you generate these classes yourself. Yeah, it makes more sense now that these classes should be exported from the protobuf package itself.

I have also fixed https://github.com/grpc/grpc-dart/issues/461#issuecomment-802808459 to remove as Any to make my suggestion work without importing implementation classes.