Comments (7)
You're absolutely right. The logic should look for all 20x statuses. It hasn't been a problem for me in practice because I find the other statuses to be rarely used, but for completeness, it's a good idea.
I'll leave the issue open for now to see if you have more questions or insight to share.
from quest.
Yes. In my original project the encoding
field was the request encoding. The response encoding should be detectable based on headers, but I never wrote that. It is a good idea to have a separate response encoding field in case the header is missing or wrong.
I don't mind having the issue open. It's a great discussion for anyone else who wants to adopt this pattern.
from quest.
OK I have been playing with this even more and now I understand - we want an :error
tuple when the response is not 200 or 204.
I have modified this code for my own purposes in two ways:
- I have change
handle_response
to allow for more configurable "success_status_codes":
@success_status_codes [200, 201]
# ...
defp handle_response({:ok, %Response{body: body, status_code: scode} = resp}) when scode in @success_status_codes do
case Jason.decode(body) do
{:ok, resp} -> {:ok, {scode, resp}}
{:error, _} -> {:error, {:json, {scode, resp}}}
end
end
#...
# other http response code
defp handle_response({:ok, %Response{status_code: scode, body: body}}) do
case Jason.decode(body) do
{:ok, json} ->
{:error, {scode, json}}
# return unprocessed body
_ ->
{:error, {:json, {scode, body}}}
end
end
- I have changed my "service" module to auto-mock in test environment!
defp maybe_mock_dispatcher do
if Application.get_env(:doorframe, :env) == :test and is_nil(System.get_env("TEST_WITH_REAL_API")) do
[dispatcher: mocked_dispatcher]
else
[]
end
end
def client(client_opts \\ []) do
maybe_mock_dispatcher()
|> Keyword.merge(client_opts)
|> Enum.into(@default_q)
end
This lets me do, .e.g user interaction testing without worrying about injecting mocks somehow. I can easily test with real API by either setting the env var TEST_WITH_REAL_API or by passing client_opts
with [dispatcher: Quest.HTTPoisonDispatcher]
.
I will post more here as I keep altering this great little "lib" (I know it's not really a lib yet).
from quest.
Hi @peaceful-james ! Thanks for your interest. I'm sorry for my delay replying, but I'm glad to see you figured things out.
Yes, the idea is if it's not a 20x status, then to our application logic it's an error, even though HTTPoison said it's :ok
because it received a response from the server.
You're doing the right thing by customizing it for your needs. This is not exactly the code I run in production. I extracted this from there and improved on some naming that is not worth refactoring in my system.
There is one thing that I had considered initially, but talked myself out of, but in the past week I wish I'd really done it -- to have handle_response
include the original Quest struct as a 3rd element of the return tuple. There's some cases I've been debugging due to a 3rd party API acting in undocumented ways that I really wanted the exact struct we sent plus the result in one place to log. That would be difficult for me to add given the size of my existing codebase, but if you're just starting out, it's something to consider.
That auto-mocking idea is interesting. Due to my "organically grown" codebase, it's not appropriate for my case, but if I were starting fresh I might consider something like that. I also have some vague notions of using the process dictionary like Mox, but until I delve into that, I'm not sure if the advantages are worth the complexity.
from quest.
Thanks for the reply @gvaughn and for the insight. I get the idea that if it's not 20x it should return :error
but actually it returns error for 201 in its current state.
Feel free to close this issue. I just wrote my impressions for the sake of posterity.
from quest.
The Stripe API requires x-www-form-urlencoded
requests but sends back json
responses.
Knowing this, the :encoding
field should be changed to :request_encoding
and :response_encoding
. I have done this in projects that use the quest pattern and hit Stripe's API.
@gvaughn You can close this issue if you want, I'm just using it to record my ideas.
from quest.
should be detectable based on headers
In an ideal world! :)
from quest.
Related Issues (2)
- Release on hex.pm HOT 6
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from quest.