dashbitco / broadway_cloud_pub_sub

A Broadway producer for Google Cloud Pub/Sub
Apache License 2.0
70 stars 24 forks source link

Error in BroadwayCloudPubSub.GoogleApiClient.wrap_received_messages/2 #34

Closed Adzz closed 5 years ago

Adzz commented 5 years ago

For some reason that I can't figure out, when querying Pub Sub in production, line 71 in GoogleApiClient the pubsub_projects_subscriptions_pull/3 returns me something that looks like this:

 {:ok,
 %GoogleApi.PubSub.V1.Model.PullResponse{
   receivedMessages: [
     %{
       "ackId" => "ID",
       "message" => %{
         "attributes" => %{"space" => "vegans"},
         "data" => "VEhJUyBJUyBBIFRFU1Q=",
         "messageId" => "1234321",
         "publishTime" => "2019-09-05T17:41:31.520Z"
       }
     }
   ]
 }}

However I notice when I run the tests here the same function returns me:

{:ok,
 %GoogleApi.PubSub.V1.Model.PullResponse{
   receivedMessages: [
     %GoogleApi.PubSub.V1.Model.ReceivedMessage{
       ackId: "ACK ID",
       message: %GoogleApi.PubSub.V1.Model.PubsubMessage{
         attributes: %{"space" => "vegans"},
         data: "VEhJUyBJUyBBIFRFU1Q=",
         messageId: "708601755676243",
         publishTime: ~U[2019-09-05 17:41:31.520Z]
       }
     }
   ]
 }}

Any ideas?

mcrumm commented 5 years ago

@Adzz Are you using the latest version of this library? There are possible incompatibilities with Poison in previous releases. The 0.4.0 release ensures your project is using a compatible version.

This seems reminiscent of googleapis/elixir-google-api#1119

Adzz commented 5 years ago

@mcrumm yes am on the latest version of this lib 0.4.0 and of broadway. Does sound suspicious though. i did check the poison versions also, which is sub 4 (3.1.0 if memory serves). We use jason in our app if that makes a difference?

I’ll triple check the mix.lock version and paste here tomorrow if that’s okay.

Adzz commented 5 years ago

@mcrumm checked the mix.lock:

"poison": {:hex, :poison, "3.1.0", "d9eb636610e096f86f25d9a46f35a9facac35609a7591b3be3326e99a0484665", [:mix], [], "hexpm"},
"broadway": {:hex, :broadway, "0.4.0", "b8daf580baed44347a9690449f9d8d7a308c1ca086648397d1a88eea893d047e", [:mix], [{:gen_stage, "~> 0.14", [hex: :gen_stage, repo: "hexpm", optional: false]}], "hexpm"},
"broadway_cloud_pub_sub": {:hex, :broadway_cloud_pub_sub, "0.4.0", "4da2d145b80669c94a6c7b62a79d29c1be9d970bce88bb9d7200bdf7d249a9a9", [:mix], [{:broadway, "~> 0.4.0", [hex: :broadway, repo: "hexpm", optional: false]}, {:google_api_pub_sub, "~> 0.11", [hex: :google_api_pub_sub, repo: "hexpm", optional: false]}, {:goth, "~> 1.0", [hex: :goth, repo: "hexpm", optional: true]}, {:hackney, "~> 1.6", [hex: :hackney, repo: "hexpm", optional: false]}], "hexpm"},

Which I think is correct?

mcrumm commented 5 years ago

Can you please post at least partial output from running GoogleApi.PubSub.V1.Api.Projects.pubsub_projects_subscriptions_pull/5 for your subscription?

I'd like to verify that this is an issue with :google_api_pub_sub, and not with our connector.

Adzz commented 5 years ago

Sure thing, the output is what I put in the issue above:

 {:ok,
 %GoogleApi.PubSub.V1.Model.PullResponse{
   receivedMessages: [
     %{
       "ackId" => "ID",
       "message" => %{
         "attributes" => %{"space" => "vegans"},
         "data" => "VEhJUyBJUyBBIFRFU1Q=",
         "messageId" => "1234321",
         "publishTime" => "2019-09-05T17:41:31.520Z"
       }
     }
   ]
 }}

which makes me think it is a poison thing, but the versions are correct as far as I can tell.

Adzz commented 5 years ago

It looks like this line is not returning what I want, so I think you are right it must be an issue in the google client: https://github.com/googleapis/elixir-google-api/blob/master/clients/gax/lib/google_api/gax/response.ex#L67

mcrumm commented 5 years ago

Sorry, I misspoke in the other issue. The struct for receivedMessages should be %GoogleApi.PubSub.V1.Model.ReceivedMessage{}, of which message is a PubsubMessage struct. So there are two layers of Poison decoding that aren't working as I would expect.

PullResponse defines nested ReceivedMessage list: https://github.com/googleapis/elixir-google-api/blob/master/clients/pub_sub/lib/google_api/pub_sub/v1/model/pull_response.ex#L36

ReceivedMessage defines nested PubsubMessage field: https://github.com/googleapis/elixir-google-api/blob/master/clients/pub_sub/lib/google_api/pub_sub/v1/model/received_message.ex#L36

Decoding for most, if not all, structs ultimately ends up in ModelBase: https://github.com/googleapis/elixir-google-api/blob/master/clients/gax/lib/google_api/gax/model_base.ex#L47

So somewhere between PullResponse and ReceivedMessage Poison stopped decoding based on the struct defined for the field 🤔

mcrumm commented 5 years ago

@Adzz Have you had any more luck resolving this issue? As it stands, I can't seem to reproduce it.

I built a sample application, mostly to help aid in testing this. Can we do some compare/contrast for dependency versions beyond what's already been listed, and any other differences that you might see? https://github.com/mcrumm/cloud_pubsub_samples/blob/master/mix.lock

/cc @wojtekmach

Adzz commented 5 years ago

@mcrumm Thanks this looks great, I will really try to get to it today. Stay tuned!

Adzz commented 5 years ago

@mcrumm Okay annoyingly, I tried it today and it just works. I tried rolling back the version of the google_api_pub_sub lib to 0.11.0, but that had no affect. So like am stumped. I guess it works so it's fine, 🤷‍♂ sorry for wasting your time, I'll close the issue.

mcrumm commented 5 years ago

I am stumped, too. I wish we could have determined the root cause of your issue, but I am glad that the latest changes are working for you.