Skip to content

Logical error for response processing#95

Open
ASnow wants to merge 1 commit into
balvig:mainfrom
ASnow:master
Open

Logical error for response processing#95
ASnow wants to merge 1 commit into
balvig:mainfrom
ASnow:master

Conversation

@ASnow

@ASnow ASnow commented Jan 10, 2018

Copy link
Copy Markdown

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

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
@coveralls

Copy link
Copy Markdown

Coverage Status

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

@balvig

balvig commented Jan 11, 2018

Copy link
Copy Markdown
Owner

@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

@ASnow

ASnow commented Jan 11, 2018

Copy link
Copy Markdown
Author

@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

@balvig

balvig commented Jan 12, 2018

Copy link
Copy Markdown
Owner

@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?

@ASnow

ASnow commented Jan 12, 2018

Copy link
Copy Markdown
Author

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

@balvig

balvig commented Feb 1, 2018

Copy link
Copy Markdown
Owner

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?

@ASnow

ASnow commented Feb 9, 2018

Copy link
Copy Markdown
Author

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

@balvig

balvig commented Feb 13, 2018

Copy link
Copy Markdown
Owner

@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?

Base automatically changed from master to main February 24, 2021 03:59
@balvig
balvig force-pushed the main branch 7 times, most recently from dac98fe to 5fa3e00 Compare December 17, 2023 06:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants