spyke icon indicating copy to clipboard operation
spyke copied to clipboard

Logical error for response processing

Open ASnow opened this issue 6 years ago • 8 comments

My case: Result return "" for #errors when DELETE method responded with no body(204 status).

Problem HashWithIndifferentAccess.new pass first argument to Hash.new that recognize it as default value for all keys. This happen when body don't repond to #to_hash

ASnow avatar Jan 10 '18 14:01 ASnow

Coverage Status

Coverage remained the same at 99.449% when pulling dea0002b5ca4518c7c246e3ae9984f85a53751d8 on ASnow:master into 52c917809b52f833b6d42de34204ac9c80604819 on balvig:master.

coveralls avatar Jan 10 '18 14:01 coveralls

@ASnow usually I will handle this at the faraday layer, as the problems that occur can differ from each user.

You can massage the returned body in anyway you want here, so that eventually is follows this format before being passed on to Spyke:

{ data: { id: 1, name: 'Bob' }, metadata: {}, errors: {} }

https://github.com/balvig/spyke#configuration

balvig avatar Jan 11 '18 02:01 balvig

@balvig, please look at Faraday default handling process https://github.com/lostisland/faraday/search?utf8=%E2%9C%93&q=parse_body%3F&type=

It don't run parse in middleware for such case

ASnow avatar Jan 11 '18 11:01 ASnow

@ASnow you could add some middleware before that, that gets rid of blank strings, or even flesh out your own JSONParser middleware to come up with your own definition of when parse_body? should be true?

balvig avatar Jan 12 '18 00:01 balvig

@balvig, yep I can. Is it ok that Result handles well(silent) when get not Hash?

ASnow avatar Jan 12 '18 07:01 ASnow

Is it ok that Result handles well(silent) when get not Hash?

@ASnow sorry, I couldn't quite catch what you mean by this question?

balvig avatar Feb 01 '18 05:02 balvig

I can describe two possible solutions for current situation.

# This is rigth silent behavior
def not_respose_with_any_object
  mock = Minitest::Mock.new
  mock.expect :respond_to?, false, [:to_hash]

  res = Spyke:: Result.new mock
  refute_same res.data, mock # this error can cause false positve method call on mock
  refute_same res.metadata, mock
  refute_same res. errors, mock
end

# This is rigth fail behavior
def raise_error_when_not_duck
  mock = Minitest::Mock.new
  mock.expect :respond_to?, false, [:to_hash]

  assert_raises ExpectValidStruct do
    Spyke:: Result.new mock
  end
end

ASnow avatar Feb 09 '18 14:02 ASnow

@ASnow could you try showing a higher level test, for example based on a Recipe.destroy (maybe you can use some of the existing tests as a reference), that shows what the problem you're encountering is?

balvig avatar Feb 13 '18 00:02 balvig