feat: first draft req adapter #190
Conversation
WalkthroughAdds an optional Req-based HTTP adapter Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ReqAdapter as ExGram.Adapter.Req
participant Req
participant Server as HTTP Server
Client->>ReqAdapter: request(method, path, opts)
ReqAdapter->>ReqAdapter: prepare base_url, coerce verb
ReqAdapter->>ReqAdapter: choose body type (multipart or json)
alt multipart
ReqAdapter->>ReqAdapter: assemble multipart parts (file, file_content, streams, kv)
ReqAdapter->>Req: set form_multipart body & headers
else json
ReqAdapter->>ReqAdapter: encode JSON body, set JSON headers
ReqAdapter->>Req: set json body
end
ReqAdapter->>ReqAdapter: attach custom_encode/custom_decode steps
Req->>Server: perform HTTP request
Server-->>Req: HTTP response
Req-->>ReqAdapter: request result
ReqAdapter->>ReqAdapter: custom_decode -> parse JSON or produce error
alt 2xx with ok:true
ReqAdapter-->>Client: {:ok, result}
else non-2xx / error
ReqAdapter-->>Client: {:error, %ExGram.Error{}}
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Potential attention points:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
lib/ex_gram/adapter/req.ex (1)
102-104: Consider atoms! to avoid atom leakage.Decoding with keys: :atoms can create atoms at runtime. Prefer keys: :atoms! or decode to strings and cast known keys.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
mix.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
lib/ex_gram/adapter/req.ex(1 hunks)lib/ex_gram/adapter/tesla.ex(1 hunks)lib/ex_gram/macros/executer.ex(1 hunks)mix.exs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
lib/ex_gram/adapter/req.ex (3)
lib/ex_gram/adapter/tesla.ex (5)
request(28-32)handle_result(50-52)handle_result(54-56)encode(79-84)filter_map(89-91)lib/ex_gram/macros/executer.ex (1)
encode(106-111)lib/ex_gram/encoder.ex (2)
encode!(56-58)decode(60-62)
🔇 Additional comments (3)
lib/ex_gram/macros/executer.ex (1)
175-177: Formatting-only change; behavior unchanged. LGTM.lib/ex_gram/adapter/tesla.ex (1)
20-21: Removal of custom encode in JSON middleware is fine; encode_body handles it.LGTM.
mix.exs (1)
59-59: No changes needed: "~> 0.5.0" covers the latest stable v0.5.15 (<0.6.0).
|
readme will look like this once merged 
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
lib/ex_gram/adapter/req.ex (2)
13-15: Parity: coerce GET to POST.Maintain Tesla behavior to avoid breaking Telegram calls that expect POST.
Apply:
- [method: verb, url: path] + [method: coerce_to_post(verb), url: path]Add helper:
+ defp coerce_to_post(:get), do: :post + defp coerce_to_post(v), do: vTo confirm Tesla parity in-repo:
#!/bin/bash rg -nC3 -P 'coerce|:get\s*->\s*:post|method:\s*verb' lib/ex_gram/adapter/tesla.ex
26-28: Multipart file part missing filename.Req doesn’t infer filename from File.stream!/2; include it to satisfy Telegram.
- defp req_parts({:file, name, path}, parts) do - parts ++ [{name, File.stream!(path, 2048)}] - end + defp req_parts({:file, name, path}, parts) do + parts ++ [{name, {File.stream!(path, 2048), filename: Path.basename(path)}}] + end
🧹 Nitpick comments (4)
lib/ex_gram/adapter/req.ex (4)
43-45: Filter nils before JSON option.Keep parity with Tesla’s filter_map and reduce payload.
- defp put_body_option(req, body) when is_map(body) do - Req.Request.put_new_option(req, :json, body) - end + defp put_body_option(req, body) when is_map(body) do + Req.Request.put_new_option(req, :json, filter_map(body)) + end
105-113: Don’t turn decode errors into exceptions; pass through on failure.Safer fallback while keeping atoms decode.
- defp custom_decode({request, response}) do - case ExGram.Encoder.decode(response.body, keys: :atoms) do - {:ok, decoded} -> - {request, put_in(response.body, decoded)} - - {:error, e} -> - {request, e} - end - end + defp custom_decode({request, %Req.Response{body: body} = response}) do + case ExGram.Encoder.decode(body, keys: :atoms) do + {:ok, decoded} -> {request, %{response | body: decoded}} + {:error, _} -> {request, response} + end + end
8-8: Remove unused Logger.- require Logger + # require Logger
12-24: Optional: add request timeouts and configurability for Req steps.
- Set receive/connect timeouts to avoid hung calls.
- Allow injecting custom Req steps via config to satisfy “middlewares” objective.
Example:
# after register_options/put_new_option |> Req.Request.put_new_option(:connect_options, [timeout: ExGram.Config.get(:ex_gram, :connect_timeout, 5_000)]) |> Req.Request.put_new_option(:receive_timeout, ExGram.Config.get(:ex_gram, :receive_timeout, 15_000)) # allow extra steps |> then(fn r -> case ExGram.Config.get(:ex_gram, :req_request_steps) do nil -> r steps when is_list(steps) -> Enum.reduce(steps, r, &Req.Request.append_request_steps(&2, &1)) end end)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/ex_gram/adapter/req.ex(1 hunks)lib/ex_gram/adapter/tesla.ex(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/ex_gram/adapter/tesla.ex
🧰 Additional context used
🧬 Code graph analysis (1)
lib/ex_gram/adapter/req.ex (3)
lib/ex_gram/adapter/tesla.ex (5)
request(30-34)handle_result(52-54)handle_result(56-58)encode(81-86)filter_map(91-93)lib/ex_gram/encoder.ex (2)
encode!(56-58)decode(60-62)lib/ex_gram/macros/executer.ex (1)
encode(106-111)
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
lib/ex_gram/adapter.ex (1)
13-21: Consider applying filter_map to plain maps and lists for consistency.The
encode/1function handles structs well (converts to map, filters nils, then encodes), but the second clause for plain maps and lists directly callsExGram.Encoder.encode!(x)without filtering. This means:
- Plain maps containing
nilvalues won't have those filtered out- Plain maps containing nested struct values won't have those structs converted to maps before encoding
This could lead to inconsistent behavior depending on whether the input is a struct or a plain map with the same data.
Consider updating the second clause to apply filtering:
-def encode(x) when is_map(x) or is_list(x), do: ExGram.Encoder.encode!(x) +def encode(x) when is_map(x) or is_list(x) do + x |> filter_map() |> ExGram.Encoder.encode!() +endThis would ensure consistent nil-filtering and nested-struct handling regardless of the input type.
lib/ex_gram/adapter/req.ex (1)
56-59: Normalize header casing for consistency.Lines 58-59 use title case (
"Content-Type","Accept") while line 53 uses lowercase ("content-type"). Although HTTP headers are case-insensitive, consistent casing improves code maintainability.Apply this diff to normalize to lowercase:
data = request.options[:json] -> %{request | body: ExGram.Adapter.encode(data)} - |> Req.Request.put_new_header("Content-Type", "application/json") - |> Req.Request.put_new_header("Accept", "application/json") + |> Req.Request.put_new_header("content-type", "application/json") + |> Req.Request.put_new_header("accept", "application/json")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
lib/ex_gram/adapter.ex(1 hunks)lib/ex_gram/adapter/req.ex(1 hunks)lib/ex_gram/adapter/tesla.ex(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-17T08:16:08.863Z
Learnt from: pxp9
PR: rockneurotiko/ex_gram#190
File: lib/ex_gram/adapter/req.ex:26-31
Timestamp: 2025-10-17T08:16:08.863Z
Learning: In ExGram's Req adapter (lib/ex_gram/adapter/req.ex), when using File.stream!/2 for multipart uploads, Req automatically extracts the filename from the File.Stream structure using Path.basename(stream.path) and sets the content_type, so no explicit filename tuple wrapper is needed.
Applied to files:
lib/ex_gram/adapter/req.ex
🧬 Code graph analysis (3)
lib/ex_gram/adapter/tesla.ex (4)
lib/ex_gram/adapter.ex (1)
encode(13-18)lib/ex_gram/encoder.ex (1)
encode(52-54)lib/ex_gram/adapter/req.ex (2)
handle_result(87-89)handle_result(91-93)lib/ex_gram/error.ex (1)
defmodule ExGram.Error do(1-5)
lib/ex_gram/adapter/req.ex (3)
lib/ex_gram/adapter.ex (1)
encode(13-18)lib/ex_gram/macros/executer.ex (1)
encode(106-111)lib/ex_gram/encoder.ex (1)
decode(60-62)
lib/ex_gram/adapter.ex (2)
lib/ex_gram/macros/executer.ex (1)
encode(106-111)lib/ex_gram/encoder.ex (2)
encode(52-54)encode!(56-58)
🔇 Additional comments (5)
lib/ex_gram/adapter/tesla.ex (2)
52-52: LGTM - Error message encoding looks correct.The use of
ExGram.Adapter.encode(body)properly converts the response body to a JSON string for the error message.
59-61: Now I need to check the actual codebase to see how the encode_body function is used and if there's any test or integration code that shows the intended behavior. Let me search for test cases and usage patterns.Based on my verification, I've confirmed the concern raised in the review comment. Let me generate the rewritten review comment:
Verify the encoding behavior to prevent potential double-encoding in HTTP requests.
The code calls
ExGram.Adapter.encode(value)on each map value before passing to Tesla's JSON middleware. Since ExGram uses Jason engine by default, and Jason.encode! returns JSON strings like "{"age":44,"name":"Steve Irwin","nationality":"Australian"}", each map value becomes a JSON-encoded string.When Tesla's JSON middleware then processes this map with string values, Tesla.Middleware.JSON converts the Elixir map into a JSON string and automatically adds the Content-Type header. This causes double-encoding: the already-JSON string values get escaped during the final encoding, resulting in incorrect nested structure encoding.
Example of the issue:
- Input struct with nested user:
{user: %{id: 123}}- After
ExGram.Adapter.encode:{user: "{\"id\":123}"}(string value)- After Tesla encoding:
{"user":"{\\\"id\\\":123}"}(escaped quotes indicate double-encoding)Fix: Either remove the pre-encoding call and pass the raw Elixir data structures to Tesla, or decode the result from
ExGram.Encoder.encode!back to Elixir structures before adding to the map. Verify that the adapter's encoding approach aligns with Tesla's expectation of receiving unencoded Elixir data structures.lib/ex_gram/adapter.ex (1)
23-40: LGTM - filter_map correctly handles nested structures.The
filter_map/1implementation properly:
- Recursively filters
nilvalues from maps- Converts nested structs to maps
- Descends into nested lists and maps
- Has appropriate base cases
The recursive logic is sound and will correctly normalize complex nested data structures.
lib/ex_gram/adapter/req.ex (2)
26-36: LGTM - Multipart part handling is correct.The multipart part construction properly handles:
- File streaming with automatic filename extraction (Req handles this via
File.Stream.path)- File content with explicit filename
- Regular form values passed through from upstream encoding
This aligns with the learning that Req automatically extracts filenames from
File.Streamstructures.Based on learnings.
82-93: LGTM - Result handling follows proper pattern.The
handle_result/1clauses correctly:
- Match successful 2xx responses with
ok: trueand extract the result- Handle error responses by encoding the body for the error message
- Handle exceptions from Req operations
The error encoding uses
ExGram.Adapter.encode(body)which properly converts the response body to a string for the error message.
| defp custom_decode({request, response}) do | ||
| case ExGram.Encoder.decode(response.body, keys: :atoms) do | ||
| {:ok, decoded} -> | ||
| {request, put_in(response.body, decoded)} | ||
|
|
||
| {:error, e} -> | ||
| {request, e} | ||
| end | ||
| end |
There was a problem hiding this comment.
Improve error handling in custom_decode.
Lines 77-79 return {request, e} when decoding fails, where e is an error tuple. However, Req response steps should return {request, response} where the second element is a Req.Response struct. Returning an error tuple could cause issues in handle_result/1 or subsequent steps.
Consider these options:
Option 1: Keep the original response on decode error
defp custom_decode({request, response}) do
case ExGram.Encoder.decode(response.body, keys: :atoms) do
{:ok, decoded} ->
{request, put_in(response.body, decoded)}
{:error, e} ->
- {request, e}
+ Logger.warning("Failed to decode response body: #{inspect(e)}")
+ {request, response}
end
endOption 2: Store decode error in response body
{:error, e} ->
- {request, e}
+ {request, %{response | body: {:decode_error, e}}}Then update handle_result/1 to check for decode errors.
🤖 Prompt for AI Agents
In lib/ex_gram/adapter/req.ex around lines 72 to 80, custom_decode currently
returns {request, e} on decode failure which breaks the Req step contract;
instead, return a Req.Response struct and record the decode failure in the
response (e.g., leave the original response but set response.body to a sentinel
like {:decode_error, error} or similar) so downstream steps still receive a
response struct; then update handle_result/1 to detect that sentinel in
response.body and handle it (log/translate to an error tuple or propagate
appropriately) so decode errors are handled consistently.
There was a problem hiding this comment.
I think this is right, you have to return {request, response} on the middleware, right?
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Req response steps return value contract
💡 Result:
Do you mean an API/request–response contract for a step (e.g., CI/workflow step), a consumer-driven contract (Pact/Spring Cloud Contract), or an OpenAPI/response schema? Pick one — or say if you want a short template for a step plugin return object, a Pact interaction, or an OpenAPI response schema.
Quick references:
- Step plugin/step-return pattern (status/message + HTTP code guidance). [1]
- HTTP-step inputs/outputs examples for workflows (fields stored as step outputs). [2][6]
- OpenAPI: how to define response schemas and media types. [7]
- Pact: consumer-driven request/response contract examples and matchers. [4][5]
Which of the three (step plugin return object, Pact contract, or OpenAPI response schema) should I produce a concrete contract/template for?
🌐 Web query:
Elixir Req library response steps API
💡 Result:
Short summary — Req response steps API
-
What a response step is: a function that receives a {request, response} tuple and must return either {request, response} or {request, exception}. Returning an exception switches to the error-step pipeline. [1]
-
What an error step is: a function that receives {request, exception} and must return either {request, exception} or {request, response}. Returning a response switches back to response steps. [1]
-
Step shapes/types (callable forms): anonymous function of the described arity or a {Module, fun, args} tuple (applied as apply(module, fun, [request | args]) ). [1]
-
How to attach/compose steps: use Req.Request.append_response_steps/prepend_response_steps (and append/prepend for request/error steps) to add named steps (keyword list) to a Req.Request struct. Example:
req = Req.Request.append_response_steps(req, decode: &MyModule.decode/1). [1] -
Built-in response-related steps: many are provided in Req.Steps (e.g., decode_body/decompress_body/follow_redirects/cache/handle_http_errors). These are normal response/request/error steps you can reuse or reorder. [2]
-
Halting: any step can halt the pipeline via Req.Request.halt/2, returning a halted request and an exception to stop further steps. [1]
Sources:
[1] Req.Request (API docs) — hexdocs.pm/req/Req.Request.html.
[2] Req.Steps (built-in steps) and README — hexdocs.pm/req/Req.Steps.html and github.com/wojtekmach/req.
@rockneurotiko Yes, you're absolutely correct! According to the Req documentation, response steps must return either {request, response} or {request, exception}. Returning an exception switches to the error-step pipeline, while returning anything else breaks the contract.
The current code on lines 77-79 returns {request, e} where e is an error tuple from the decoder (not a Req.Response struct), which violates this contract and could cause issues in handle_result/1 or subsequent steps.
Recommended fix:
I'd suggest Option 1 from CodeRabbit's comment (keeping the original response on decode error) as it's simpler and safer:
defp custom_decode({request, response}) do
case ExGram.Encoder.decode(response.body, keys: :atoms) do
{:ok, decoded} ->
{request, put_in(response.body, decoded)}
{:error, e} ->
Logger.warning("Failed to decode response body: #{inspect(e)}")
{request, response}
end
endThis way, handle_result/1 will receive the original response body (likely a string) and can handle it appropriately. If you need to distinguish decode errors downstream, Option 2 (storing {:decode_error, e} in the response body) would work, but then you'd need to update handle_result/1 to check for that pattern.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
lib/ex_gram/adapter/req.ex (2)
12-12: Add GET→POST coercion for parity with Tesla adapter.The Tesla adapter coerces
:getverbs to:postfor Telegram API compatibility, but the Req adapter uses the verb directly. This behavioral difference could cause issues.Apply this diff to add coercion:
+ defp coerce_verb(:get), do: :post + defp coerce_verb(verb), do: verb + @impl ExGram.Adapter def request(verb, path, body) do - [method: verb, url: path] + [method: coerce_verb(verb), url: path] |> Req.Request.new()
70-78: Fix error handling to maintain response step contract.When decoding fails, line 76 returns
{request, e}whereeis an error tuple{:error, reason}. However, Req response steps must return{request, %Req.Response{}}tuples. Returning an error tuple breaks the contract and could cause pattern match failures inhandle_result/1.Apply this diff to preserve the response structure:
defp custom_decode({request, response}) do case ExGram.Encoder.decode(response.body, keys: :atoms) do {:ok, decoded} -> {request, put_in(response.body, decoded)} {:error, e} -> - {request, e} + # Keep original binary body on decode failure + # handle_result will process it as an error + {request, response} end endThis allows
handle_result/1to consistently receive aReq.Responsestruct and handle the raw body as an error case.
🧹 Nitpick comments (1)
lib/ex_gram/adapter/req.ex (1)
51-51: Normalize header casing for consistency.Line 51 uses lowercase
"content-type"while lines 56-57 use title case"Content-Type"and"Accept". Although HTTP headers are case-insensitive, consistent casing improves code readability.Consider normalizing to lowercase throughout:
- |> Req.Request.put_new_header("Content-Type", "application/json") - |> Req.Request.put_new_header("Accept", "application/json") + |> Req.Request.put_new_header("content-type", "application/json") + |> Req.Request.put_new_header("accept", "application/json")Also applies to: 56-57
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/ex_gram/adapter/req.ex(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-17T08:16:08.863Z
Learnt from: pxp9
PR: rockneurotiko/ex_gram#190
File: lib/ex_gram/adapter/req.ex:26-31
Timestamp: 2025-10-17T08:16:08.863Z
Learning: In ExGram's Req adapter (lib/ex_gram/adapter/req.ex), when using File.stream!/2 for multipart uploads, Req automatically extracts the filename from the File.Stream structure using Path.basename(stream.path) and sets the content_type, so no explicit filename tuple wrapper is needed.
Applied to files:
lib/ex_gram/adapter/req.ex
🧬 Code graph analysis (1)
lib/ex_gram/adapter/req.ex (2)
lib/ex_gram/adapter.ex (2)
encode(13-18)defmodule ExGram.Adapter do(1-12)lib/ex_gram/encoder.ex (1)
decode(60-62)
🔇 Additional comments (5)
lib/ex_gram/adapter/req.ex (5)
1-2: LGTM: Clean conditional compilation.The conditional loading pattern correctly handles Req as an optional dependency.
24-34: LGTM: Multipart part handling is correct.The file streaming and form value handling are properly implemented. Based on learnings, Req automatically extracts filenames from File.Stream structures, and upstream encoding ensures values are in the correct format.
36-43: LGTM: Body option handling is clean.The multipart and JSON body routing logic is clear and correct.
64-68: LGTM: Nil-safe content-length handling.The guard clause properly handles optional content-length values.
80-94: LGTM: Result handling covers expected cases.The pattern matches correctly handle successful responses, error responses, and exceptions. Once the
custom_decode/1issue is resolved, this function will work correctly.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
lib/ex_gram/adapter/req.ex (1)
70-78: Fix custom_decode to return valid Req response tuple.On decode error, line 76 returns
{request, e}whereeis an error tuple, violating Req's step contract. Response steps must return{request, response}where the second element is aReq.Responsestruct, or{request, exception}to switch to error handling.Apply this diff to keep the original response on decode failure:
defp custom_decode({request, response}) do case ExGram.Encoder.decode(response.body, keys: :atoms) do {:ok, decoded} -> {request, put_in(response.body, decoded)} {:error, e} -> - {request, e} + require Logger + Logger.warning("Failed to decode response body: #{inspect(e)}") + {request, response} end endThis ensures
handle_result/1always receives a validReq.Responsestruct.
🧹 Nitpick comments (1)
lib/ex_gram/adapter/req.ex (1)
56-57: Consider normalizing header casing for consistency.The multipart section uses lowercase headers (
"content-type","content-length"), while the JSON section uses title case ("Content-Type","Accept"). While HTTP headers are case-insensitive, consistent casing improves code maintainability.- |> Req.Request.put_new_header("Content-Type", "application/json") - |> Req.Request.put_new_header("Accept", "application/json") + |> Req.Request.put_new_header("content-type", "application/json") + |> Req.Request.put_new_header("accept", "application/json")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
assets/ex_gram_logo.pngis excluded by!**/*.pngmix.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
README.md(1 hunks)lib/ex_gram/adapter.ex(1 hunks)lib/ex_gram/adapter/req.ex(1 hunks)lib/ex_gram/adapter/tesla.ex(2 hunks)lib/ex_gram/macros/executer.ex(1 hunks)mix.exs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/ex_gram/macros/executer.ex
- mix.exs
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: pxp9
Repo: rockneurotiko/ex_gram PR: 190
File: lib/ex_gram/adapter/req.ex:26-31
Timestamp: 2025-10-17T08:16:08.863Z
Learning: In ExGram's Req adapter (lib/ex_gram/adapter/req.ex), when using File.stream!/2 for multipart uploads, Req automatically extracts the filename from the File.Stream structure using Path.basename(stream.path) and sets the content_type, so no explicit filename tuple wrapper is needed.
Learnt from: rockneurotiko
Repo: rockneurotiko/ex_gram PR: 190
File: lib/ex_gram/adapter/req.ex:11-22
Timestamp: 2025-10-27T12:43:44.875Z
Learning: In the ExGram codebase, always use POST for Telegram Bot API requests, even when the Telegram documentation specifies GET. This simplifies parameter handling by avoiding URL encoding and is supported by the Telegram API. Both Tesla and Req adapters should coerce :get verbs to :post.
📚 Learning: 2025-10-17T08:16:08.863Z
Learnt from: pxp9
Repo: rockneurotiko/ex_gram PR: 190
File: lib/ex_gram/adapter/req.ex:26-31
Timestamp: 2025-10-17T08:16:08.863Z
Learning: In ExGram's Req adapter (lib/ex_gram/adapter/req.ex), when using File.stream!/2 for multipart uploads, Req automatically extracts the filename from the File.Stream structure using Path.basename(stream.path) and sets the content_type, so no explicit filename tuple wrapper is needed.
Applied to files:
lib/ex_gram/adapter/req.exlib/ex_gram/adapter/tesla.ex
📚 Learning: 2025-10-27T12:43:44.875Z
Learnt from: rockneurotiko
Repo: rockneurotiko/ex_gram PR: 190
File: lib/ex_gram/adapter/req.ex:11-22
Timestamp: 2025-10-27T12:43:44.875Z
Learning: In the ExGram codebase, always use POST for Telegram Bot API requests, even when the Telegram documentation specifies GET. This simplifies parameter handling by avoiding URL encoding and is supported by the Telegram API. Both Tesla and Req adapters should coerce :get verbs to :post.
Applied to files:
lib/ex_gram/adapter/req.exlib/ex_gram/adapter/tesla.ex
📚 Learning: 2025-07-01T12:23:11.101Z
Learnt from: manuel-rubio
Repo: rockneurotiko/ex_gram PR: 183
File: lib/ex_gram/dispatcher.ex:84-88
Timestamp: 2025-07-01T12:23:11.101Z
Learning: `ExGram.Token.fetch/1` in the ex_gram library returns `String.t() | nil`, not a tuple like `{:ok, token} | {:error, reason}`. When pattern matching in `with` statements, use `token when is_binary(token) <- ExGram.Token.fetch(bot: state.name)` to check for a valid token.
Applied to files:
lib/ex_gram/adapter/req.exlib/ex_gram/adapter/tesla.ex
🧬 Code graph analysis (3)
lib/ex_gram/adapter/req.ex (2)
lib/ex_gram/adapter.ex (1)
encode(13-18)lib/ex_gram/encoder.ex (1)
decode(60-62)
lib/ex_gram/adapter.ex (2)
lib/ex_gram/macros/executer.ex (1)
encode(108-113)lib/ex_gram/encoder.ex (3)
encode(52-54)encode!(56-58)defmodule ExGram.Encoder do(1-67)
lib/ex_gram/adapter/tesla.ex (3)
lib/ex_gram/adapter.ex (1)
encode(13-18)lib/ex_gram/encoder.ex (1)
encode(52-54)lib/ex_gram/adapter/req.ex (2)
handle_result(87-90)handle_result(92-94)
🪛 GitHub Actions: Elixir CI
lib/ex_gram/adapter/req.ex
[error] 78-90: Elixir formatter check failed. The file is not formatted according to mix format. Run 'mix format' to fix code style issues in this file.
lib/ex_gram/adapter/tesla.ex
[error] 80-82: Elixir formatter check failed. The file is not formatted according to mix format. Run 'mix format' to fix code style issues in this file.
🔇 Additional comments (5)
README.md (1)
3-4: LGTM! Nice documentation improvement.The centered logo adds a professional touch to the README.
lib/ex_gram/adapter/tesla.ex (2)
54-54: LGTM! Proper use of centralized encoding.Using
ExGram.Adapter.encode(body)for error message construction aligns with the shared encoding utilities introduced in this PR.
62-62: LGTM! Consistent encoding approach.The refactor to use
ExGram.Adapter.encode(value)removes code duplication and leverages the centralized encoding logic.lib/ex_gram/adapter.ex (2)
13-21: LGTM! Well-designed encoding utilities.The
encode/1functions provide a clean, public API that:
- Handles structs by converting to maps and filtering nils before encoding
- Directly encodes maps/lists
- Passes through other types unchanged
This centralized approach eliminates duplication between adapters.
23-40: LGTM! Robust nil-filtering implementation.The
filter_map/1functions correctly:
- Handle structs by converting to maps and recursing
- Remove nil values from maps while preserving nested structures
- Recursively process lists and nested maps
- Pass through non-collection types
The recursive filtering ensures clean payloads for the Telegram API.
| end | ||
|
|
||
| defp handle_result( | ||
| {_req, %Req.Response{status: status, body: %{ok: true, result: body}} = _response} | ||
| ) | ||
| when status in 200..299 do | ||
| {:ok, body} | ||
| end | ||
|
|
||
| defp handle_result({_req, %Req.Response{body: body} = _response}) do | ||
| {:error, | ||
| %ExGram.Error{code: :response_status_not_match, message: ExGram.Adapter.encode(body)}} | ||
| end |
There was a problem hiding this comment.
Fix formatting issues.
The pipeline reports formatting violations in this range. Run mix format to resolve.
#!/bin/bash
# Verify formatting issues
cd lib/ex_gram/adapter
mix format --check-formatted req.ex🧰 Tools
🪛 GitHub Actions: Elixir CI
[error] 78-90: Elixir formatter check failed. The file is not formatted according to mix format. Run 'mix format' to fix code style issues in this file.
🤖 Prompt for AI Agents
In lib/ex_gram/adapter/req.ex around lines 78 to 90, the code in the
handle_result clauses is failing mix format checks; reformat the file (or just
this function block) to conform to Elixir formatter rules — run mix format
(e.g., cd lib/ex_gram/adapter && mix format req.ex) or apply the formatter
changes: align pipeline/when clauses and maps, ensure consistent indentation and
spacing around function heads and guards, so the file passes mix format
--check-formatted.
|
@pxp9 thanks for you work. @rockneurotiko When will it merged into the master branch? |
|
I'll try to merge it next week, it's been busy on work and my life |
|
Merged! Thanks @pxp9 !!!! 🧡🧡🧡 I will do a release ASAP |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
lib/ex_gram/adapter/req.ex (1)
27-42: Consider avoiding quadratic list building inreq_parts/put_body_option.
req_parts/2usesparts ++ [...]inside a reduce, which is O(n²) in the number of parts. It’s harmless for small payloads, but easy to make linear by prepending and reversing once:- defp req_parts({:file, name, path}, parts) do - parts ++ [{name, File.stream!(path, 2048)}] - end + defp req_parts({:file, name, path}, parts) do + [{name, File.stream!(path, 2048)} | parts] + end @@ - defp req_parts({:file_content, name, content, filename}, parts) do - parts ++ [{name, {content, filename: filename}}] - end + defp req_parts({:file_content, name, content, filename}, parts) do + [{name, {content, filename: filename}} | parts] + end @@ - defp req_parts({name, value}, parts) do - parts ++ [{name, value}] - end + defp req_parts({name, value}, parts) do + [{name, value} | parts] + end @@ - defp put_body_option(req, {:multipart, parts}) do - parts = Enum.reduce(parts, [], fn part, acc -> req_parts(part, acc) end) - Req.Request.put_new_option(req, :form_multipart, parts) - end + defp put_body_option(req, {:multipart, parts}) do + parts = + parts + |> Enum.reduce([], fn part, acc -> req_parts(part, acc) end) + |> Enum.reverse() + + Req.Request.put_new_option(req, :form_multipart, parts) + endThis preserves part order while improving asymptotic behaviour.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/ex_gram/adapter/req.ex(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: pxp9
Repo: rockneurotiko/ex_gram PR: 190
File: lib/ex_gram/adapter/req.ex:26-31
Timestamp: 2025-10-17T08:16:08.863Z
Learning: In ExGram's Req adapter (lib/ex_gram/adapter/req.ex), when using File.stream!/2 for multipart uploads, Req automatically extracts the filename from the File.Stream structure using Path.basename(stream.path) and sets the content_type, so no explicit filename tuple wrapper is needed.
Learnt from: rockneurotiko
Repo: rockneurotiko/ex_gram PR: 190
File: lib/ex_gram/adapter/req.ex:11-22
Timestamp: 2025-10-27T12:43:44.875Z
Learning: In the ExGram codebase, always use POST for Telegram Bot API requests, even when the Telegram documentation specifies GET. This simplifies parameter handling by avoiding URL encoding and is supported by the Telegram API. Both Tesla and Req adapters should coerce :get verbs to :post.
📚 Learning: 2025-10-17T08:16:08.863Z
Learnt from: pxp9
Repo: rockneurotiko/ex_gram PR: 190
File: lib/ex_gram/adapter/req.ex:26-31
Timestamp: 2025-10-17T08:16:08.863Z
Learning: In ExGram's Req adapter (lib/ex_gram/adapter/req.ex), when using File.stream!/2 for multipart uploads, Req automatically extracts the filename from the File.Stream structure using Path.basename(stream.path) and sets the content_type, so no explicit filename tuple wrapper is needed.
Applied to files:
lib/ex_gram/adapter/req.ex
📚 Learning: 2025-10-27T12:43:44.875Z
Learnt from: rockneurotiko
Repo: rockneurotiko/ex_gram PR: 190
File: lib/ex_gram/adapter/req.ex:11-22
Timestamp: 2025-10-27T12:43:44.875Z
Learning: In the ExGram codebase, always use POST for Telegram Bot API requests, even when the Telegram documentation specifies GET. This simplifies parameter handling by avoiding URL encoding and is supported by the Telegram API. Both Tesla and Req adapters should coerce :get verbs to :post.
Applied to files:
lib/ex_gram/adapter/req.ex
📚 Learning: 2025-07-01T12:23:11.101Z
Learnt from: manuel-rubio
Repo: rockneurotiko/ex_gram PR: 183
File: lib/ex_gram/dispatcher.ex:84-88
Timestamp: 2025-07-01T12:23:11.101Z
Learning: `ExGram.Token.fetch/1` in the ex_gram library returns `String.t() | nil`, not a tuple like `{:ok, token} | {:error, reason}`. When pattern matching in `with` statements, use `token when is_binary(token) <- ExGram.Token.fetch(bot: state.name)` to check for a valid token.
Applied to files:
lib/ex_gram/adapter/req.ex
🧬 Code graph analysis (1)
lib/ex_gram/adapter/req.ex (4)
lib/ex_gram/adapter/tesla.ex (3)
request(31-35)handle_result(53-55)handle_result(57-59)lib/ex_gram/adapter.ex (1)
encode(13-18)lib/ex_gram/macros/executer.ex (1)
encode(108-113)lib/ex_gram/encoder.ex (1)
decode(60-62)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test (28.1.1, 1.19.2, true)
🔇 Additional comments (3)
lib/ex_gram/adapter/req.ex (3)
1-22: Req request pipeline and GET→POST coercion look correct.The guarded module definition plus
request/3pipeline (coercing:getto:post, configuring:base_url, and wiring custom encode/decode +run_request/handle_result) is consistent with the existing adapter behaviour and the “always POST to Telegram” practice. I don’t see correctness issues here.Based on learnings, this matches the project’s GET→POST convention.
48-71: custom_encode correctly produces binary bodies for multipart and JSON.The request step now lets Req’s multipart helper build the body and headers, and uses
ExGram.Adapter.encode/1to turn JSON maps into a single encoded binary with appropriate headers. That resolves the earlier risk of sending a map as the HTTP body and keeps semantics aligned with the Tesla adapter.
73-94: custom_decode + handle_result error mapping are coherent with Req’s contracts.Decoding the binary body once via
ExGram.Encoder.decode/2and returning{request, response}on success (sohandle_result/1sees%Req.Response{body: %{ok: true, result: body}}) matches the library’s expectations. On decode or transport errors, returning{request, e}and letting the finalhandle_result/1clause wrap it into%ExGram.Error{code: exception}is a reasonable, consistent strategy with the existing adapter API.
|
Released in version 0.57.0 |
3 participants
Working in progress
Req Adapter for Exgram
closes #189
Summary by CodeRabbit
New Features
Refactor / Bug Fixes
Style
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.