Heisenbug: periodic error TextMarshaler error about open-api HOT 22 OPEN

@jeffreydwalter wow amazing. I'll pull it in.

from open-api.

mmmm this is strange. I'll take a look at the api to make sure it's returning the right thing. My hunch at first glance is that the api is returning an error that doesn't follow the error convention.

from open-api.

Getting this a lot more often tonight... usually when running all tests (which are now up in the dozens). And it seems to only be the latter tests.

Could this perhaps be some sort of rate limiting error that the Netlify API client is not reporting properly? Terraform can make an awful lot of API calls in a very short period of time and I just now realized that I didn't have to configure any backoff parameters so unsure if that's related or not. Just an idea.

Here is a snippet example below. Note that each test probably makes ~5 API calls. So we're looking at around 50 API calls in less than 10 seconds for this test suite snippet below.

=== RUN   TestProvider
--- PASS: TestProvider (0.00s)
=== RUN   TestProvider_impl
--- PASS: TestProvider_impl (0.00s)
=== RUN   TestAccDeployKey_basic
--- PASS: TestAccDeployKey_basic (3.99s)
=== RUN   TestAccDeployKey_disappears
--- PASS: TestAccDeployKey_disappears (2.36s)
=== RUN   TestAccHook
--- FAIL: TestAccHook (0.71s)
	testing.go:513: Step 0 error: Error applying: 1 error(s) occurred:

		* netlify_site.test: 1 error(s) occurred:

		* netlify_site.test: &{0 } (*models.Error) is not supported by the TextConsumer, can be resolved by supporting TextUnmarshaler interface
=== RUN   TestAccHook_disappears
--- FAIL: TestAccHook_disappears (0.62s)
	testing.go:513: Step 0 error: Error applying: 1 error(s) occurred:

		* netlify_site.test: 1 error(s) occurred:

		* netlify_site.test: &{0 } (*models.Error) is not supported by the TextConsumer, can be resolved by supporting TextUnmarshaler interface
=== RUN   TestAccHook_updateData
--- FAIL: TestAccHook_updateData (0.60s)
	testing.go:513: Step 0 error: Error applying: 1 error(s) occurred:

		* netlify_site.test: 1 error(s) occurred:

		* netlify_site.test: &{0 } (*models.Error) is not supported by the TextConsumer, can be resolved by supporting TextUnmarshaler interface
=== RUN   TestAccSite_basic
--- FAIL: TestAccSite_basic (0.55s)
	testing.go:513: Step 0 error: Error applying: 1 error(s) occurred:

		* netlify_site.test: 1 error(s) occurred:

		* netlify_site.test: &{0 } (*models.Error) is not supported by the TextConsumer, can be resolved by supporting TextUnmarshaler interface
=== RUN   TestAccSite_disappears
--- FAIL: TestAccSite_disappears (0.49s)
	testing.go:513: Step 0 error: Error applying: 1 error(s) occurred:

		* netlify_site.test: 1 error(s) occurred:

		* netlify_site.test: &{0 } (*models.Error) is not supported by the TextConsumer, can be resolved by supporting TextUnmarshaler interface
=== RUN   TestAccSite_updateName
--- FAIL: TestAccSite_updateName (0.49s)
	testing.go:513: Step 0 error: Error applying: 1 error(s) occurred:

		* netlify_site.test: 1 error(s) occurred:

		* netlify_site.test: &{0 } (*models.Error) is not supported by the TextConsumer, can be resolved by supporting TextUnmarshaler interface

from open-api.

It's possible that this is a rate limiting error, I'm going to dive into the logs. Can you try enabling debug mode? Setting os.Setenv("DEBUG", "1") should make open-api to log all the request/response interactions raw, which might let you see the response object.

from open-api.

@calavera Confirmed! 😊

HTTP/2.0 429 Too Many Requests
Content-Length: 70
Age: 0
Content-Type: text/plain; charset=utf-8
Date: Wed, 20 Dec 2017 17:22:52 GMT
Server: Netlify
X-Ratelimit-Limit: 3
X-Ratelimit-Remaining: -2
X-Ratelimit-Reset: 2017-12-20 17:23:52 UTC

{"code":429,"error":"API Deploy rate limit surpassed for application"}

Many API clients we work with in Terraform implement handling of the X-Ratelimit-* headers themselves (with options for backoff and so on). Is that something you're interested in? In the mean time we can probably do something...

In any case one issue is that that error isn't being parsed properly into an error message!

from open-api.

I asked our TF providers team and they said that the API client pretty much always handles this in some way. They gave me a couple examples to share if you're interested.

Azure: Azure actually does a really fantastic job. Retries are pretty much invisible and just work. They use go-autorest under the hood and here is the relevant code: https://github.com/Azure/go-autorest/blob/809ed2ef5c4c9a60c3c2f3aa9cc11f3a7c2ce59d/autorest/sender.go#L208 The only tunable they expose is "MaxRetries" and we just use the default.

AWS: The AWS client uses a Handlers struct which is a callback for various stages of the API request. One of those callbacks is Retry. But by default the AWS client handles the rate limit, waits the necessary window, and retries the request: https://github.com/terraform-providers/terraform-provider-aws/blob/cba83a50b79c55183b918152cd94aed1eaac8c98/vendor/github.com/aws/aws-sdk-go/aws/request/handlers.go#L10 And setting the handlers: https://github.com/terraform-providers/terraform-provider-aws/blob/cba83a50b79c55183b918152cd94aed1eaac8c98/aws/config.go#L437

from open-api.

Thanks for the info, those examples are super useful. I agree that we should handle the retries in the client.

from open-api.

This started happening again in some circumstances around timed out uploadFile calls. Re-opening to track and investigate.

from open-api.

Here's an example in the wild: https://app.netlify.com/sites/devopsdays-web/deploys/5c7dbf8c591fc60008c4fd86

We have almost nothing happening in our deploy that should take much time. Last week we had timeouts, and this started happening today. Happy to work with you if you want to try to reproduce/test with us.

/cc @mattstratton

from open-api.

Thank you @bridgetkromhout! We've identified the underlying issue and are working on a fix. Once that's out, I'll dig around and figure out why errors aren't propagating correctly (to figure out what's causing the awkward TextMarshaler error to be what you are seeing, instead of something actually useful).

from open-api.

The offending parameter in the response seems to be application/json missing from Content-Type. So its super important our endpoints set that header. It seems like this is sometimes not the case, especially when things are going wrong.

Asking about the TextConsumer error in go-swagger's slack I received this tip:

it should only do that if there is no default content type set
at the global level in the spec

Which we do set if my understanding is correct. This could be an error in the open-api runtime, or we aren't setting a definition correctly in our spec, however I suspect its a bug or some error handling in need of improvement.

from open-api.

Tips to fix upstream:

ic I just checked our templates
yeah we don't implement the text marshaller
it's probably very similar to the binary stuff though
https://github.com/go-swagger/go-swagger/blob/master/generator/templates/schema.gotmpl#L215-L233

from open-api.

As far as I am aware, its when we don't receive a content type of json, and it appeared to be an upstream generator issue. I intended to look into it, but ran out of time in the window I had.

from open-api.

I believe I might have fixed this issue. See my issue here: go-swagger/go-swagger#1917

from open-api.

@bcomnes let me know how it goes. Should be enough to just update the openapi/runtime package.

from open-api.

@jeffreydwalter I don't think it does quite solve the issue, but I'll take the updates! The issue I think we are running into here is twofold:

• If the API you are talking to respondes in a weird way, say, returns a non json content type, you get a NoConsumer error. While this is the cause of the error, it would be better if the client returned a better error along the lines of, Bad response, error code etc.
• If the API should return JSON and it isn't, the API should be fixed to return JSON.

from open-api.

@bcomnes sorry for the false hope. 🙃 In my case I was dealing with issue number two. I had a third party API that was returning an empty body and text/plain in response to a 302 after a POST. The spec only had application/json specified for the content-type. Since the body was empty, the client was trying to unmarshal it as plain text and failing.

from open-api.

I think ultimately, that message is due to the fact that the API is returning a content type that the spec doesn't define. The spec and/or the API need fixing.
You can see in this comment that the content type is text/plain: #58 (comment)

The openapi/runtime code is trying to cast the request data data.(encoding.TextUnmarshaler) and failing since models.Error doesn't implement the TextUnmarshaler interface. See: https://github.com/go-openapi/runtime/blob/3d6572c5a80826d5e7f6362d4c681875207c3ef6/text.go#L48

from open-api.

Its been a while, but iirc the ideas I had were:

• Support encoding.TextUnmarshaler in the Error template.
• Wrap the no-consumer error to return a better top-level error. While no consumer IS the underlying problem, the consumer of the client should get an error that more closely resembles a server error or bad response error (e.g. what was the route, what was the code, what was the content type etc).

from open-api.

sorry for the false hope

No problem at all. I appreciate the pings on possibly helpful updates and it sounds like you DID fix something that maybe affects us but I haven't realized yet.

from open-api.

Can someone unassigned me on this? I can't for some reason.

from open-api.

Error: Deploy did not succeed: Failed to execute deploy: &{0 } (*models.Error) is not supported by the TextConsumer, can be resolved by supporting TextUnmarshaler interface

Seeing error like this when I tried to deploy just now

from open-api.