mirror of
https://github.com/we-promise/sure.git
synced 2026-05-29 15:34:58 +00:00
* fix(plaid): surface configuration/product-access errors from Link flow (#1792) * fix(plaid): harden Plaid Link onExit guard + nil-body JSON parse (#1792 review) * fix lint check issue * fix test unit check
This commit is contained in:
committed by
Juan José Mata
parent
4acf3ea581
commit
fb50963794
@@ -1,4 +1,6 @@
|
||||
class PlaidItemsController < ApplicationController
|
||||
include StreamExtensions
|
||||
|
||||
before_action :set_plaid_item, only: %i[edit destroy sync]
|
||||
before_action :require_admin!, only: %i[new create select_existing_account link_existing_account edit destroy sync]
|
||||
|
||||
@@ -12,6 +14,8 @@ class PlaidItemsController < ApplicationController
|
||||
accountable_type: params[:accountable_type] || "Depository",
|
||||
region: region
|
||||
)
|
||||
rescue Plaid::ApiError => e
|
||||
handle_link_token_error(e)
|
||||
end
|
||||
|
||||
def edit
|
||||
@@ -21,6 +25,8 @@ class PlaidItemsController < ApplicationController
|
||||
webhooks_url: webhooks_url,
|
||||
redirect_url: accounts_url,
|
||||
)
|
||||
rescue Plaid::ApiError => e
|
||||
handle_link_token_error(e)
|
||||
end
|
||||
|
||||
def create
|
||||
@@ -104,6 +110,58 @@ class PlaidItemsController < ApplicationController
|
||||
plaid_item_params.dig(:metadata, :institution, :name)
|
||||
end
|
||||
|
||||
# When `link_token/create` (or the update equivalent) raises, surface a
|
||||
# friendly alert to the user instead of letting the modal frame render
|
||||
# blank. Plaid configuration/product-access errors are the common case for
|
||||
# self-hosted users — without this, the Link modal simply never opens and
|
||||
# the only signal lives in server logs.
|
||||
def handle_link_token_error(error)
|
||||
error_body = safe_parse_plaid_error(error)
|
||||
error_code = error_body["error_code"].to_s
|
||||
|
||||
Rails.logger.warn(
|
||||
"Plaid link_token request failed: #{error_code} - #{error_body['error_message']}"
|
||||
)
|
||||
Sentry.capture_exception(error) if defined?(Sentry)
|
||||
|
||||
alert = friendly_link_token_alert(error_code, error_body["error_message"])
|
||||
|
||||
respond_to do |format|
|
||||
format.html { redirect_to accounts_path, alert: alert }
|
||||
format.turbo_stream { stream_redirect_to(accounts_path, alert: alert) }
|
||||
end
|
||||
end
|
||||
|
||||
def safe_parse_plaid_error(error)
|
||||
JSON.parse(error.response_body.to_s)
|
||||
rescue JSON::ParserError
|
||||
{}
|
||||
end
|
||||
|
||||
# Plaid surfaces its own actionable copy on configuration / product-access
|
||||
# failures (e.g. "Your account is not enabled for the following products
|
||||
# [...]. To request access, visit dashboard.plaid.com..."). Those messages
|
||||
# are safe to show verbatim — they describe a Plaid-side config issue,
|
||||
# not user data. For everything else we fall back to a generic message
|
||||
# and rely on the log + Sentry trail.
|
||||
SHOWABLE_PLAID_ERROR_CODES = %w[
|
||||
INVALID_PRODUCT
|
||||
PRODUCTS_NOT_SUPPORTED
|
||||
NO_PRODUCTS_PERMISSION
|
||||
ADDITION_LIMIT
|
||||
INVALID_INSTITUTION
|
||||
INSTITUTION_NOT_ENABLED_IN_REGION
|
||||
INSTITUTION_NOT_SUPPORTED
|
||||
].freeze
|
||||
|
||||
def friendly_link_token_alert(error_code, error_message)
|
||||
if SHOWABLE_PLAID_ERROR_CODES.include?(error_code) && error_message.present?
|
||||
t("plaid_items.errors.link_token_with_message", message: error_message)
|
||||
else
|
||||
t("plaid_items.errors.link_token_generic")
|
||||
end
|
||||
end
|
||||
|
||||
def plaid_us_webhooks_url
|
||||
return webhooks_plaid_url if Rails.env.production?
|
||||
|
||||
|
||||
@@ -128,9 +128,32 @@ export default class extends Controller {
|
||||
};
|
||||
|
||||
handleExit = (err, metadata) => {
|
||||
// If there was an error during update mode, refresh the page to show latest status
|
||||
if (err && metadata.status === "requires_credentials") {
|
||||
// If there was an error during update mode, refresh the page to show
|
||||
// latest status. Guard `metadata` (Plaid can fire onExit with it
|
||||
// undefined when Link aborts very early) and gate the redirect on
|
||||
// `isUpdateValue` so first-time link failures don't bounce the user
|
||||
// away from whatever page they were on.
|
||||
if (
|
||||
err &&
|
||||
metadata &&
|
||||
metadata.status === "requires_credentials" &&
|
||||
this.isUpdateValue
|
||||
) {
|
||||
window.location.href = "/accounts";
|
||||
return;
|
||||
}
|
||||
|
||||
// Promote Plaid's own error payload to the console so a silent modal
|
||||
// close still leaves a breadcrumb (issue #1792). Plaid Link's own UI
|
||||
// is responsible for showing a message inside the modal when this
|
||||
// fires; backend link-token failures are handled server-side via the
|
||||
// PlaidItemsController rescue + flash.
|
||||
if (err?.error_code) {
|
||||
console.error(
|
||||
"Plaid Link exited with error",
|
||||
err.error_code,
|
||||
err.display_message || err.error_message
|
||||
);
|
||||
}
|
||||
};
|
||||
|
||||
|
||||
@@ -45,15 +45,24 @@ class PlaidItem < ApplicationRecord
|
||||
access_token: access_token
|
||||
)
|
||||
rescue Plaid::ApiError => e
|
||||
error_body = JSON.parse(e.response_body)
|
||||
|
||||
if error_body["error_code"] == "ITEM_NOT_FOUND"
|
||||
# Mark the connection as invalid but don't auto-delete
|
||||
update!(status: :requires_update)
|
||||
error_body = begin
|
||||
JSON.parse(e.response_body.to_s)
|
||||
rescue JSON::ParserError
|
||||
{}
|
||||
end
|
||||
|
||||
Sentry.capture_exception(e)
|
||||
nil
|
||||
if error_body["error_code"] == "ITEM_NOT_FOUND"
|
||||
# Mark the connection as invalid but don't auto-delete. The caller
|
||||
# gets nil so the calling controller can decide what to render.
|
||||
update!(status: :requires_update)
|
||||
Sentry.capture_exception(e) if defined?(Sentry)
|
||||
nil
|
||||
else
|
||||
# Re-raise so the controller can surface a friendly alert to the user
|
||||
# (issue #1792). Swallowing here previously left the Plaid modal frame
|
||||
# blank with no actionable signal.
|
||||
raise
|
||||
end
|
||||
end
|
||||
|
||||
def destroy_later
|
||||
|
||||
@@ -5,6 +5,10 @@ en:
|
||||
success: Account linked successfully. Please wait for accounts to sync.
|
||||
destroy:
|
||||
success: Accounts scheduled for deletion.
|
||||
errors:
|
||||
link_token_generic: We couldn't open Plaid right now. Please try again, and
|
||||
if the problem persists check the server logs for details.
|
||||
link_token_with_message: "Plaid couldn't open the connection: %{message}"
|
||||
plaid_item:
|
||||
add_new: Add new connection
|
||||
confirm_accept: Delete institution
|
||||
|
||||
@@ -6,6 +6,58 @@ class PlaidItemsControllerTest < ActionDispatch::IntegrationTest
|
||||
sign_in @user = users(:family_admin)
|
||||
end
|
||||
|
||||
test "new redirects with friendly alert when Plaid rejects link_token request for unauthorized products" do
|
||||
# Reproduces issue #1792: the Plaid client account isn't enabled for the
|
||||
# requested products, so Plaid returns an actionable error message. We
|
||||
# should surface that message instead of letting the modal frame render
|
||||
# blank.
|
||||
plaid_provider = mock
|
||||
Provider::Registry.stubs(:plaid_provider_for_region).with(:us).returns(plaid_provider)
|
||||
|
||||
error_body = {
|
||||
"error_code" => "INVALID_PRODUCT",
|
||||
"error_message" => "Your account is not enabled for the following products: [\"investments\" \"liabilities\" \"transactions\"]. To request access, visit https://dashboard.plaid.com/overview/request-products or contact Sales or your Account Manager."
|
||||
}.to_json
|
||||
plaid_provider.expects(:get_link_token).raises(
|
||||
Plaid::ApiError.new(code: 400, response_body: error_body)
|
||||
)
|
||||
|
||||
get new_plaid_item_url(accountable_type: "Investment")
|
||||
|
||||
assert_redirected_to accounts_path
|
||||
assert_match(/not enabled for the following products/, flash[:alert])
|
||||
end
|
||||
|
||||
test "new redirects with generic alert when Plaid raises an unclassified error" do
|
||||
plaid_provider = mock
|
||||
Provider::Registry.stubs(:plaid_provider_for_region).with(:us).returns(plaid_provider)
|
||||
|
||||
plaid_provider.expects(:get_link_token).raises(
|
||||
Plaid::ApiError.new(code: 500, response_body: { "error_code" => "INTERNAL_SERVER_ERROR" }.to_json)
|
||||
)
|
||||
|
||||
get new_plaid_item_url
|
||||
|
||||
assert_redirected_to accounts_path
|
||||
assert_equal I18n.t("plaid_items.errors.link_token_generic"), flash[:alert]
|
||||
end
|
||||
|
||||
test "edit redirects with friendly alert when Plaid rejects update link_token request" do
|
||||
plaid_item = plaid_items(:one)
|
||||
error_body = {
|
||||
"error_code" => "INVALID_PRODUCT",
|
||||
"error_message" => "Your account is not enabled for the following products: [\"transactions\"]."
|
||||
}.to_json
|
||||
PlaidItem.any_instance.expects(:get_update_link_token).raises(
|
||||
Plaid::ApiError.new(code: 400, response_body: error_body)
|
||||
)
|
||||
|
||||
get edit_plaid_item_url(plaid_item)
|
||||
|
||||
assert_redirected_to accounts_path
|
||||
assert_match(/not enabled for the following products/, flash[:alert])
|
||||
end
|
||||
|
||||
test "create" do
|
||||
@plaid_provider = mock
|
||||
Provider::Registry.expects(:plaid_provider_for_region).with("us").returns(@plaid_provider)
|
||||
|
||||
@@ -44,4 +44,46 @@ class PlaidItemTest < ActiveSupport::TestCase
|
||||
@plaid_item.destroy
|
||||
end
|
||||
end
|
||||
|
||||
test "get_update_link_token marks item as requires_update and returns nil on ITEM_NOT_FOUND" do
|
||||
error_response = { "error_code" => "ITEM_NOT_FOUND", "error_message" => "not found" }.to_json
|
||||
Family.any_instance.expects(:get_link_token).raises(
|
||||
Plaid::ApiError.new(code: 400, response_body: error_response)
|
||||
)
|
||||
|
||||
result = @plaid_item.get_update_link_token(webhooks_url: "https://x", redirect_url: "https://x")
|
||||
|
||||
assert_nil result
|
||||
assert_predicate @plaid_item.reload, :requires_update?
|
||||
end
|
||||
|
||||
test "get_update_link_token re-raises other Plaid errors so the controller can surface them" do
|
||||
# Issue #1792: silently swallowing all Plaid errors here is what made the
|
||||
# "modal closes with nothing happening" experience so opaque.
|
||||
error_response = { "error_code" => "INVALID_PRODUCT", "error_message" => "Your account is not enabled..." }.to_json
|
||||
Family.any_instance.expects(:get_link_token).raises(
|
||||
Plaid::ApiError.new(code: 400, response_body: error_response)
|
||||
)
|
||||
|
||||
assert_raises(Plaid::ApiError) do
|
||||
@plaid_item.get_update_link_token(webhooks_url: "https://x", redirect_url: "https://x")
|
||||
end
|
||||
assert_predicate @plaid_item.reload, :good?
|
||||
end
|
||||
|
||||
test "get_update_link_token tolerates a Plaid::ApiError with a nil/blank response_body" do
|
||||
# Plaid clients have been observed raising ApiError without a response
|
||||
# body (network-layer failures, early aborts). The old JSON.parse would
|
||||
# blow up with TypeError before the rescue could fire; we now coerce
|
||||
# to String so the parse falls back to {} and the error re-raises
|
||||
# cleanly for the controller to handle.
|
||||
Family.any_instance.expects(:get_link_token).raises(
|
||||
Plaid::ApiError.new(code: 500, response_body: nil)
|
||||
)
|
||||
|
||||
assert_raises(Plaid::ApiError) do
|
||||
@plaid_item.get_update_link_token(webhooks_url: "https://x", redirect_url: "https://x")
|
||||
end
|
||||
assert_predicate @plaid_item.reload, :good?
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user