Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Error message is lost when publishing invalid events #116

Closed
GJL opened this issue Dec 23, 2016 · 5 comments
Closed

Error message is lost when publishing invalid events #116

GJL opened this issue Dec 23, 2016 · 5 comments
Labels
bug
Milestone
0.8.0

Comments

@GJL
Copy link

GJL commented Dec 23, 2016 •
edited
Loading

Nakadi returns an array instead of a single object when schema violations are found. Find example below.

[{"publishing_status":"failed","detail":"#: 2 schema violations found\n#: ... }]

However, OkHttpResource.handleError() expects a single Problem object:

Problem problem = (Problem)Optional.ofNullable(this.jsonSupport.fromJson(raw, Problem.class)) ...

Hence, the deserialization with gson fails with the following exception:

Caused by: java.lang.IllegalStateException: Expected BEGIN_OBJECT but was BEGIN_ARRAY at line 1 column 2 path $
	at nakadi.shadow.com.google.gson.stream.JsonReader.beginObject(JsonReader.java:385)
	at nakadi.shadow.com.google.gson.internal.bind.ReflectiveTypeAdapterFactory$Adapter.read(ReflectiveTypeAdapterFactory.java:213)
	... 49 more
@dehora
Copy link
Owner

dehora commented Jan 12, 2017

@GJL This is an issue with the Nakadi implementation; it doesn't match its API. I'd suggest filing a bug upstream in Nakadi. Will leave this open for the time being.

@GJL
Copy link
Author

GJL commented Jan 23, 2017 •
edited
Loading

@dehora Sorry, I am on vacation and only now had time to reply.

https://github.com/zalando/nakadi/blob/master/api/nakadi-event-bus-api.yaml#L377

 '422':
          description: |
            At least one event failed to be validated, enriched or partitioned. None were submitted.
          schema:
            type: array
            items:
              $ref: '#/definitions/BatchItemResponse'

To me it seems that the behavior is documented correctly in the Nakadi project: The return type can be an array.

@dehora
Copy link
Owner

dehora commented Jan 30, 2017 •
edited
Loading

To me it seems that the behavior is documented correctly in the Nakadi project: The return type can be an array.

Got it. The server's also returning the wrong format for some errors, which distracted me. Marking this as a bug in batch handling.

@dehora dehora added bug and removed help wanted labels Jan 30, 2017
@dehora dehora modified the milestone: 0.8.0 Jan 31, 2017
dehora added a commit that referenced this issue Feb 13, 2017
@dehora
Handle batch item responses for event posting.
1e0af05 
This supports the case when the server returns 207 or 422 for an
event post. It also provides a convenience method on EventResource
that returns a BatchItemResponseCollection instead of having to
marshal the Response content from the other send methods.

For #116.
@dehora dehora mentioned this issue Feb 13, 2017
Merged
dehora added a commit that referenced this issue Feb 13, 2017
@dehora
Handle batch item responses for event posting.
ea75c25 
This supports the case when the server returns 207 or 422 for an
event post. It also provides a convenience method on EventResource
that returns a BatchItemResponseCollection instead of having to
marshal the Response content from the other send methods.

For #116.
dehora added a commit that referenced this issue Feb 13, 2017
@dehora
Handle batch item responses for event posting.
935a042 
This supports the case when the server returns 207 or 422 for an
event post. It also provides a convenience method on EventResource
that returns a BatchItemResponseCollection instead of having to
marshal the Response content from the other send methods.

For #116.
dehora added a commit that referenced this issue Feb 13, 2017
@dehora
Handle batch item responses for event posting.
166fad7 
This supports the case when the server returns 207 or 422 for an
event post. It also provides a convenience method on EventResource
that returns a BatchItemResponseCollection instead of having to
marshal the Response content from the other send methods.

For #116.
@dehora
Copy link
Owner

dehora commented Feb 13, 2017

There's a patch for this in #122 which has been released as 0.7.5

@dehora
Copy link
Owner

dehora commented Feb 14, 2017

Had a report from a customer it's working for them in 0.7.5, closing

@dehora dehora closed this as completed Feb 14, 2017
@dehora dehora mentioned this issue May 2, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug
Projects
None yet
Milestone
0.8.0
Development

No branches or pull requests
2 participants
@dehora @GJL