Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warning on using Sugar on more recent versions of Elixir #86

Closed
CharlesOkwuagwu opened this issue Feb 25, 2016 · 7 comments
Closed

Warning on using Sugar on more recent versions of Elixir #86

CharlesOkwuagwu opened this issue Feb 25, 2016 · 7 comments

Comments

@CharlesOkwuagwu
Copy link

Hi, we get a few warnings when compiling Sugar on more recent versions of Elixir.
I believe these can be avoided as the warnings suggest some simple solutions:

D:\Elixir>cd simple

D:\Elixir\simple>mix
Compiled lib/simple.ex
lib/simple/router.ex:2: warning: the variable "parsers_opts" is unsafe as it has been set in a conditional clause, as part of a case/cond/receive/if/&&/||. Please rewrite the clauses so the value is explicitly returned. For example:

    case int do
      1 -> atom = :one
      2 -> atom = :two
    end

Can be rewritten as:

    atom =
      case int do
        1 -> :one
        2 -> :two
      end

Compiled lib/pages/controllers/pages.ex
Compiled lib/simple/controllers/hello.ex
Compiled lib/simple/controllers/main.ex
lib/simple/router.ex:1: warning: the variable "opts" is unsafe as it has been set in a conditional clause, as part of a case/cond/receive/if/&&/||. Please rewrite the clauses so the value is explicitly returned. For example:

    case int do
      1 -> atom = :one
      2 -> atom = :two
    end

Can be rewritten as:

    atom =
      case int do
        1 -> :one
        2 -> :two
      end
...
...

Compiled lib/simple/router.ex
Generated simple app
Consolidated DBConnection.Query
Consolidated Poison.Decoder
Consolidated Plug.Exception
Consolidated List.Chars
Consolidated Ecto.DataType
Consolidated Ecto.Queryable
Consolidated Collectable
Consolidated Enumerable
Consolidated String.Chars
Consolidated IEx.Info
Consolidated Poison.Encoder
Consolidated Inspect
deps/templates/lib/sugar/templates/engines/eex.ex:13: warning: variable assigns is unused
deps/templates/lib/sugar/templates/engines/eex.ex:13: warning: redefining module :"Elixir.Sugar.Templates.CompiledTemplates.EEx.index.html.eex" (current version defined in memory)
deps/templates/lib/sugar/templates/engines/eex.ex:13: warning: variable assigns is unused

D:\Elixir\simple>
@YellowApple
Copy link
Member

Are you using the "simple" app template (https://github.com/sugar-framework/simple-example)? If so, it's worth mentioning that said template hasn't been updated in almost a year (note to self: fix that). Notably, you'll want to update Sugar from 0.4.6 to 0.4.10 (in your mix.exs).

More to the point: do the warnings persist if you create a fresh Sugar project (following the getting started guide)?

@CharlesOkwuagwu
Copy link
Author

No. Simple is just a name I choose for my project:

D:\Elixir>mix new simple

Also loading the latest version of Sugar

  defp deps do
    [
      #{ :sugar, "~> 0.4.6" }
      # Comment out the above line and uncomment the below one if you want the latest/greatest
       {:sugar, github: "sugar-framework/sugar"}
    ]
  end

@CharlesOkwuagwu
Copy link
Author

It all works fine now, but this comes up each time you refresh the index page: localhost:4000

D:\Elixir\simple>iex --werl -S mix server

IEx(3)>
09:24:55.303 [error] #PID<0.431.0> running Simple.Router terminated
Server: localhost:4000 (http)
Request: GET /
** (exit) an exception was raised:
    ** (Plug.Conn.AlreadySentError) the response was already sent
        (plug) lib/plug/conn.ex:343: Plug.Conn.send_resp/1
        (simple) lib/simple/controllers/main.ex:1: Simple.Controllers.Main.call/2
        (simple) lib/simple/router.ex:1: Simple.Router.do_call/2
        (plug) lib/plug/adapters/cowboy/handler.ex:15: Plug.Adapters.Cowboy.Handler.upgrade/4
        (cowboy) src/cowboy_protocol.erl:442: :cowboy_protocol.execute/4

My router is very basic, hence I think this is coming from either use Sugar.Router or use Sugar.Controller

defmodule Simple.Router do
  use Sugar.Router
  plug Sugar.Plugs.HotCodeReload

  if Sugar.Config.get(:sugar, :show_debugger, false) do
    use Plug.Debugger, otp_app: :sugar_test
  end

  plug Plug.Static, at: "/static", from: :sugar_test

  # Uncomment the following line for session store
  # plug Plug.Session, store: :ets, key: "sid", secure: true, table: :session

  # Note the use of Plug.Parsers, and the use of the actual parsing
  # module (Plug.Parsers.MULTIPART) instead of an atom (:multipart)
  plug Plug.Parsers, parsers: [ Plug.Parsers.MULTIPART ]

  # Define your routes here
  get "/", Simple.Controllers.Main, :index
  post "/upload", Simple.Controllers.Main, :upload  # Note the POST
end

My controller is similarly very basic:

defmodule Simple.Controllers.Main do
  use Sugar.Controller

  def index(conn, []) do
    raw conn |> render #resp(200, "<h1>Hello guys</h1>")
  end

  def upload(conn, _) do
    require Logger
    Logger.debug(conn |> inspect)

    case conn.params["image"] do
      %Plug.Upload{filename: filename, path: path} ->
        {:ok, image} = File.read path
        {:ok, file} = File.open filename, [:write]
        IO.binwrite file, image
        File.close file

        conn |> render(filename: filename)  # Note use of render, not resp
      nil ->
        raw conn |> resp(400, "image was not attached!")
    end
  end
end

@YellowApple
Copy link
Member

That usually happens on a refresh or page load immediately following a source code change. Not sure about a root cause yet beyond Sugar.Plugs.HotCodeReload. You can comment out line 2 of your router if it bugs you (at the expense of your app requiring a restart whenever you want a change to take effect), but the error shouldn't actually affect anything.

Closing this issue since the original problem is resolved. I've opened a new issue (sugar-framework/plugs#5) to track the Plug.Conn.AlreadySentError on code reload.

@CharlesOkwuagwu
Copy link
Author

@YellowApple Erm, the linked above issue is totally unrelated to this problem.

Please recheck the "new" issue you opened related to this.

Thanks

@YellowApple
Copy link
Member

Whoops. Meant sugar-framework/plugs#5. Sorry about that. Must've had some dyslexic moment there...

@CharlesOkwuagwu
Copy link
Author

LOL, no worries. cheers.

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

No branches or pull requests

2 participants