Skip to content

Commit

Permalink
Handle missing trip info gracefully
Browse files Browse the repository at this point in the history
FinderApi.trip was not accounting for the possibility that the
`trip_info` it looks up might be nil, leading to crashes. We also add a
bit of logging to help debug the ultimate reason that the `trip_info`
can be nil.
  • Loading branch information
phildarnowsky committed Mar 6, 2020
1 parent 029a8da commit 1d5f9df
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 5 deletions.
28 changes: 23 additions & 5 deletions apps/site/lib/site_web/controllers/schedule/finder_api.ex
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ defmodule SiteWeb.ScheduleController.FinderApi do

import SiteWeb.ScheduleController.ScheduleApi, only: [format_time: 1, fares_for_service: 4]

require Logger

@type react_keys :: :date | :direction | :is_current
@type react_strings :: [{react_keys, String.t()}]
@type converted_values :: {Date.t(), integer, boolean}
Expand Down Expand Up @@ -101,6 +103,8 @@ defmodule SiteWeb.ScheduleController.FinderApi do
opts = Map.get(conn.assigns, :trip_info_functions, [])
params = %{"origin" => origin, "trip" => trip_id}

original_query_params = conn.query_params

trip_info =
conn
|> assign(:date, service_end_date)
Expand All @@ -112,12 +116,26 @@ defmodule SiteWeb.ScheduleController.FinderApi do
|> Trips.call(Trips.init(opts))
|> Map.get(:assigns)
|> Map.get(:trip_info)
|> add_computed_fares_to_trip_info(route)
|> json_safe_trip_info()
|> update_in([:times], &add_delays/1)
|> update_in([:times], &simplify_time/1)

json(conn, trip_info)
if trip_info do
trip_info =
trip_info
|> add_computed_fares_to_trip_info(route)
|> json_safe_trip_info()
|> update_in([:times], &add_delays/1)
|> update_in([:times], &simplify_time/1)

json(conn, trip_info)
else
_ =
Logger.warn(
"trip_info_not_found original_query_params=#{Jason.encode!(original_query_params)}"
)

conn
|> put_resp_content_type("application/json")
|> send_resp(404, "null")
end
end

# Use internal API to generate list of relevant schedules and predictions
Expand Down
24 changes: 24 additions & 0 deletions apps/site/test/site_web/controllers/schedule/finder_api_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,30 @@ defmodule SiteWeb.ScheduleController.FinderApiTest do
assert %{"times" => [processed_prediction | _]} = response
assert %{"prediction" => %{"time" => nil}} = processed_prediction
end

test "doesn't 500 if trip info not found", %{conn: conn} do
date = Util.service_date() |> Date.to_iso8601()

path =
finder_api_path(conn, :trip, %{
id: "",
route: "CR-Providence",
direction: "0",
date: date,
stop: "place-sstat"
})

opts = [
trip_fn: fn _, _ -> [@schedule] end,
prediction_fn: fn _ -> [@prediction] end
]

_ =
conn
|> assign(:trip_info_functions, opts)
|> get(path)
|> json_response(404)
end
end

describe "maybe_add_delay/1" do
Expand Down

0 comments on commit 1d5f9df

Please sign in to comment.