mschae / trailing_format_plug

An elixir plug to support legacy APIs that use a rails-like trailing format: http://api.dev/resources.json
25 stars 15 forks source link

Breaks non-existent routes with format extensions #10

Open ecoslado opened 7 years ago

ecoslado commented 7 years ago

We are currently using trailing_format_plug for an API we are implementing with Phoenix. We use it to declare urls like example.com/:one_param, receive requests with example.com/one_value.js and use "one_value" in the action.

When favicon.ico is requested, a FunctionClauseError is raised (NotFound was expected). This is the error message: (FunctionClauseError) no function clause matching in Plug.Conn.resp/3

I thought it could be fixed by adding an option :only to TrailingFormatPlug, just to list which formats you allow to be set by TrailingFormatPlug, but I'm new in Elixir-Phoenix ecosystem so I'd like to know if there's a better way...

mschae commented 7 years ago

Can you post the entire stacktrace please?

ecoslado commented 7 years ago

Sure, here it is. Thanks a lot.

2016-12-25 12:23:45.127 [error] #PID<0.3679.0> running Doophoenix.Endpoint terminated
Server: localhost:4000 (http)
Request: GET /favicon.ico
** (exit) an exception was raised:
    ** (FunctionClauseError) no function clause matching in Plug.Conn.resp/3
        (plug) lib/plug/conn.ex:493: Plug.Conn.resp(%Plug.Conn{adapter: {Plug.Adapters.Cowboy.Conn, :...}, assigns: %{kind: :error, layout: false, reason: %Phoenix.Router.NoRouteError{conn: %Plug.Conn{adapter: {Plug.Adapters.Cowboy.Conn, :...}, assigns: %{start: -576460663683269924}, before_send: [#Function<0.60513273/1 in Doophoenix.Plug.Logger.call/2>], body_params: %{}, cookies: %Plug.Conn.Unfetched{aspect: :cookies}, halted: false, host: "localhost", method: "GET", owner: #PID<0.3679.0>, params: %{"_format" => "ico"}, path_info: ["favicon"], path_params: %{}, peer: {{127, 0, 0, 1}, 49957}, port: 4000, private: %{Doophoenix.Router => {[], %{}}, :phoenix_endpoint => Doophoenix.Endpoint, :phoenix_pipelines => [], :phoenix_router => Doophoenix.Router, :plug_route => #Function<1.35251107/1 in TrailingFormatPlug.do_match/4>}, query_params: %{"_format" => "ico"}, query_string: "", remote_ip: {127, 0, 0, 1}, req_cookies: %Plug.Conn.Unfetched{aspect: :cookies}, req_headers: [{"host", "localhost:4000"}, {"connection", "keep-alive"}, {"cache-control", "max-age=0"}, {"upgrade-insecure-requests", "1"}, {"user-agent", "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/55.0.2883.95 Safari/537.36"}, {"accept", "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8"}, {"accept-encoding", "gzip, deflate, sdch, br"}, {"accept-language", "en-US,en;q=0.8,es;q=0.6"}, {"cookie", "optimizelyEndUserId=oeu1453291252012r0.17206413461826742; session-set=true; bsUl=0; bsCoId=1000001249004; sId=0; ai_user=djZ++|2016-02-03T07:51:38.533Z; adsp_di=; hasConsent=true; _cb_ls=1; s_fid=75AD857C3D4CF138-36C98CB7FC93EF31; s_lv=1455700086959; cX_P=ikqm8vsys2irzhpy; _m_u_2=212f2638-d556-11e5-9d2a-07e57f9ec93d; _bb=56c438bf9485d27a85ce62d2; _bb_t=%5B%5D; _m_u=893853c9-158f-4bef-9094-1699fae9c872; aceptacionCK=1; _chartbeat2=Cd-YsUPIe_KiCIFn.1455641510032.1456238618119.11000001; buySAFEUID=BSUID%3A%3A4733906f-3756-42f0-95bb-3608730272a4_56; Tawk_56b48be55d8a6c387d75d80f=vs11.tawk.to:443||0; __gfp_64b=Gw3be8mTiElC.D9jw22KQbHLG6EFaClEUuSjL8Gs_Q3.o7; _shopify_y=44A29313-20E9-45A2-90F7; _shopify_uniq=x; showLocationChange=shown; _ym_uid=1463559836741863246; Comm100_CC_Identity_60401=-14686; __atuvc=8%7C20; kameleoonVisit=in76txm9llos65yo/1/1466069151394/20160316/6905852272/true; pnctest=1; wp-settings-time-1=1467978494; TW_VISITOR_ID=8be8bc57-4f66-41c6-be4a-644cbd8e10d0; WUF=eJwNytsRABAMBMCSPI4jyonEjBqM3tnv3ScOOBqkdzNkryazOqFOUqSorpAKM_hLuw9Flwzt; _bkrmku=%7B%22user%22:%7B%22language%22:%22ES%22,%22currency%22:%22EUR%22%7D%7D; __as_rng=320; tc_priv_ok=1; TC_OPTOUT=0@@@008@@@ALL; cached_data=undefined; _#uid=1466163807263.1861853322.9132762.5855.1441243772.7; _#sess=1468497503%7C20160714130938%7C20; _#srchist=62824%3A1%3A20160718114327%7C62824%3A1468497503%3A20160814115823; _#vdf=62824%7C1468497503%7C20160814115823; olfsk=olfsk2998089869506657; hblid=qrJeipL8P2DukUuP2c89GRubOkD1BESa; tc_cj_v2=%5Ecl_%5Dny%5B%5D%5D_mmZZZZZZKNPRPKJRMQNJLZZZ%5D; PRIVACY_MOB=22; ABTasty=ABTastyUTMB%3A1%5E%7C%5ELiwioTracking%3A16020817081024249%5E%7C%5EsegmentationTracking%3A16020817081024249%5E%7C%5ELiwioUTMA%3A0.5.1454947690575.1468402046333.1468610485666.2; _iub_cs-7855961=%7B%22consent%22%3Atrue%2C%22timestamp%22%3A%222016-07-18T09%3A21%3A49.687Z%22%2C%22version%22%3A%221.1.3%22%2C%22id%22%3A7855961%7D; MCEvilPopupClosed=yes; cookie_ue=1; __insp_wid=1594204219; __insp_nv=true; __insp_ref=d; __insp_targlpu=http%3A%2F%2Flocalhost%3A8888%2Fmykaramelli.html; __insp_targlpt=Comprar%20Fondant%20Online%20-%20Utensilios%20de%20Reposter%C3%ADa; __insp_norec_sess=true; __insp_slim=1472633035796; es_newssubscriber=1; __ar_v4=LAKFQLGRKNGADMKBF3Y6KT%3A20160928%3A2%7CITPMBYKAFJBW5GX5QFHDOX%3A20160928%3A2%7CO4PMW55PYZC4PEGX2UG5OW%3A20160928%3A2; PAPVisitorId=112c086a66101963575f037b2g4CCESn; rrrbt=; rcuid=57ed0e146c7d3c1e38f5437a; rrpusid=57ed0e146c7d3c1e38f5437a; rrlpuid=; rrpvid=298774139116910; stopSubscriptionPopUp=shown; PaquetTelecom=yes; casinoApi_store_code=CG850; casinoApi_store_depot=A31850; _vwo_uuid_v2=8D4D31495F5B19D19B27E390B2FD6554|ee7d188e200912969933813bc1ce7023; accept_cookies=true; scarab.visitor=%223B899E779B36B8E0%22; optimizelySegments=%7B%22188049923%22%3A%22false%22%2C%22188080683%22%3A%22direct%22%2C%22188117527%22%3A%22none%22%2C%22188127138%22%3A%22gc%22%2C%22357640474%22%3A%22direct%22%2C%22357740462%22%3A%22false%22%2C%22357810445%22%3A%22gc%22%2C%222193960282%22%3A%22gc%22%2C%222215770366%22%3A%22false%22%2C%222225402683%22%3A%22direct%22%2C%224721955593%22%3A%22direct%22%2C%224757582418%22%3A%22gc%22%2C%224766090014%22%3A%22false%22%7D; optimizelyBuckets=%7B%7D; __tawkuuid=e::localhost::VMr0fHyEFi3AFCkhHVOE3cKJPy72uLpltNQtjQv4WF1hfW6RBo8D4gUkRQDUrhBx::2; Tawk_57f633ea6339c4365abecc69=vs42.tawk.to::0; NoticeCookie=1; SQLiteManager_currentLangue=2; ntwk=direct; _ga=GA1.1.290909441.1453291254; _oa_views_9XllxdGkWezHwzuYxqfFIg=1; _oa_block_9XllxdGkWezHwzuYxqfFIg=block; catAccCookies=true; __utma=111872281.290909441.1453291254.1482146302.1482146302.1; __utmc=111872281; __utmz=111872281.1482146302.1.1.utmcsr=(direct)|utmccn=(direct)|utmcmd=(none); csrftoken=M8Fup4LPkqNDI4MeeHS9SwzACBT7Xge6; sessionid=au58snh6jg5d0ndzn8wuk6p5bkemo0zq"}], request_path: "/favicon.ico", resp_body: nil, resp_cookies: %{}, resp_headers: [{"cache-control", "max-age=0, private, must-revalidate"}, {"x-request-id", "1cpgett4trr4rskd42l2e1p4vcha0gp1"}], scheme: :http, script_name: [], secret_key_base: "plI4FIUOalWsVs6lM3qnOs8YSr9oRI9XYnInqsWvDFv6uab1U/lHhYa2qpJqF0Y3", state: :unset, status: nil}, message: "no route found for GET /favicon (Doophoenix.Router)", plug_status: 404, router: Doophoenix.Router}, stack: [{Doophoenix.Router, :match_route, 4, [file: 'web/router.ex', line: 1]}, {Doophoenix.Router, :do_call, 2, [file: 'web/router.ex', line: 1]}, {Doophoenix.Router, :call, 2, [file: 'lib/plug/error_handler.ex', line: 64]}, {Doophoenix.Endpoint, :phoenix_pipeline, 1, [file: 'lib/doophoenix/endpoint.ex', line: 1]}, {Doophoenix.Endpoint, :"call (overridable 3)", 2, [file: 'lib/doophoenix/endpoint.ex', line: 1]}, {Plug.Adapters.Cowboy.Handler, :upgrade, 4, [file: 'lib/plug/adapters/cowboy/handler.ex', line: 15]}, {:cowboy_protocol, :execute, 4, [file: 'src/cowboy_protocol.erl', line: 442]}], start: -576460663683269924}, before_send: [#Function<0.60513273/1 in Doophoenix.Plug.Logger.call/2>], body_params: %{}, cookies: %Plug.Conn.Unfetched{aspect: :cookies}, halted: false, host: "localhost", method: "GET", owner: #PID<0.3679.0>, params: %{"_format" => "ico"}, path_info: ["favicon"], path_params: %{}, peer: {{127, 0, 0, 1}, 49957}, port: 4000, private: %{Doophoenix.Router => {[], %{}}, :phoenix_endpoint => Doophoenix.Endpoint, :phoenix_layout => false, :phoenix_pipelines => [], :phoenix_router => Doophoenix.Router, :phoenix_template => "404.ico", :phoenix_view => Doophoenix.ErrorView, :plug_route => #Function<1.35251107/1 in TrailingFormatPlug.do_match/4>}, query_params: %{"_format" => "ico"}, query_string: "", remote_ip: {127, 0, 0, 1}, req_cookies: %Plug.Conn.Unfetched{aspect: :cookies}, req_headers: [{"host", "localhost:4000"}, {"connection", "keep-alive"}, {"cache-control", "max-age=0"}, {"upgrade-insecure-requests", "1"}, {"user-agent", "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/55.0.2883.95 Safari/537.36"}, {"accept", "text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8"}, {"accept-encoding", "gzip, deflate, sdch, br"}, {"accept-language", "en-US,en;q=0.8,es;q=0.6"}, {"cookie", "optimizelyEndUserId=oeu1453291252012r0.17206413461826742; session-set=true; bsUl=0; bsCoId=1000001249004; sId=0; ai_user=djZ++|2016-02-03T07:51:38.533Z; adsp_di=; hasConsent=true; _cb_ls=1; s_fid=75AD857C3D4CF138-36C98CB7FC93EF31; s_lv=1455700086959; cX_P=ikqm8vsys2irzhpy; _m_u_2=212f2638- (truncated)
2016-12-25 12:23:46.374 [error] #PID<0.3681.0> running Doophoenix.Endpoint terminated
ecoslado commented 7 years ago

I was trying some solutions, restricting urls by configuration: https://github.com/ecoslado/trailing_format_plug/blob/master/lib/trailing_format_plug.ex

After check it out, I think it's better put all the "routing stuff" in my app and leave your plug as it is right now, so I didn't do PR.

Anyway, I'm open to ideas.

mschae commented 7 years ago

This does not seem related to the trailing_format_plug. I assume that when you delete the plug, the error is still there, is that correct?

ecoslado commented 7 years ago

It is. I think I'm missing something. I declare the plug in the endpoint, before the router:

defmodule Doophoenix.Endpoint do
  use Phoenix.Endpoint, otp_app: :doophoenix
  use PrePlug

  @logger_plug Application.fetch_env!(:doophoenix, :logger_plug)

  # Code reloading can be explicitly enabled under the
  # :code_reloader configuration of your endpoint.
  if code_reloading? do
    plug Phoenix.CodeReloader
  end
  # TrailingFormatPlug is needed to remove the .js of the hashid parameter
  # on the options call: /5/options/6a96504dc173514cab1e0198af92e6e9.js
  plug Plug.RequestId
  pre_plug @logger_plug

  plug Plug.Parsers,
    parsers: [:urlencoded, :multipart, :json],
    pass: ["*/*"],
    json_decoder: Poison

  plug Plug.MethodOverride
  plug Plug.Head

  # Using Corsica here as pre_plug makes that all errors (from request
  # validation, etc) have the access-control-allow-origin header as *
  # This way you can see the real error, and it's not masked by a
  # CORS authorization error.
  pre_plug Corsica, origins: "*"
  plug TrailingFormatPlug
  plug Doophoenix.Router
end

Because I need to declare the route with no trailing format extension. Do you think I have to declare it in the router? I need it to execute before the router don't I? As I told I'm new in Phoenix. Could you help me to get it properly working?

mschae commented 7 years ago

So this is not related to this plug and therefore this is not the right platform to debug your problem, I'm sorry.

Generally if there isn't a trailing format this plug does nothing. So /foo/bar will not be changed at all by this plug, only /foo/bar.json will.

As to your error I'd advise to go to elixirforum.com to get help (or Stack Overflow).

ecoslado commented 7 years ago

Sorry, I insist. I'm convinced the problem is related with TrailingFormatPlug.

That's why I proposed to limit the urls where the plug is applied, cause I guess the plug must be executed before the router (where I declare the route with no extension). So the "two pipelines" solution doesn't work for me.

mschae commented 7 years ago

Getting more reports, will check into this. Limiting the routes this works on doesn't feel like the right way to go tho.