Incompatibility with devise authenticate routes #358
Comments
|
Hi, thanks for reporting this. Users have worked with devise in the past, we have also done some devise integration. It looks like Can you print the |
Here's a diff: use Rack::Cors
use ActionDispatch::HostAuthorization
use Rack::Sendfile
use ActionDispatch::Static
+use AppMap::Middleware::RemoteRecording
use ActionDispatch::Executor
use ActionDispatch::ServerTiming
use ActiveSupport::Cache::Strategy::LocalCache::Middleware
use Rack::Runtime
use Rack::MethodOverride
use ActionDispatch::RequestId
use RequestStore::Middleware
use ActionDispatch::RemoteIp
use Rails::Rack::Logger
use ActionDispatch::ShowExceptions
use WebConsole::Middleware
use ActionDispatch::DebugExceptions
use Bugsnag::Rack
use ActionDispatch::ActionableExceptions
use ActionDispatch::Reloader
use ActionDispatch::Callbacks
use ActiveRecord::Migration::CheckPending
use ActionDispatch::Cookies
use ActionDispatch::Session::CookieStore
use ActionDispatch::Flash
use ActionDispatch::ContentSecurityPolicy::Middleware
use ActionDispatch::PermissionsPolicy::Middleware
use Rack::Head
use Rack::ConditionalGet
use Rack::ETag
use Rack::TempfileReaper
use Warden::Manager
use Bullet::Rack
run MyApp::Application.routes |
|
Well, I'm learning how devise works from this :-) By applying this constraint, Rails actually, essentially hides the route if the user is not authenticated. I am not sure that this is how Rails authors intended for routing constraints to be used but, here we are :-). AppMap is using the router here because it's detecting the normalized path in a way that understands Rails engines. It looks to me like it's safe to ignore exceptions that occur while testing the routes for this purpose. Here's a branch that ignores this error, can you check if it resolves this issue? #359 |
It does, thanks! |
|
@dividedmind do you think we need to do something smarter here, or is this sufficient? |
|
@kgilpin I'm not sure, won't this leave the normalized route unresolved? I'm also a bit concerned that it swallows the exception silently and unconditionally. It'll mask any other issues that might arise there. |
|
So, in this case because the user is not authenticated, Rails will actually report this as a 404 so essentially the route does not exist. I think it’s weird that devise works this way but I’m not sure what else to do about it. I suppose we could warn |
My concern is whether it won't still fail to resolve a normalized route even when the user is authenticated. |
|
I see. So maybe we should build the authenticated route at the end of the request instead of at the beginning. |
|
FYI, there's some sort of actions trigger related to opening an issue that's failing |
That's a good idea. I'll open an issue. In the meantime, I think we should go ahead with your quick fix, showing a warning and with a TODO comment in the code, so we remember to remove it when we implement the proper solution. |
Thanks! @kgilpin I believe the token is expired; I don't have access to rotate it. |
|
I've dropped that workflow and updated the PR. Take a look... |
## [1.0.1](v1.0.0...v1.0.1) (2024-04-30) ### Bug Fixes * Handle exceptions in app.matches? ([0e9db0d](0e9db0d)), closes [#358](#358)
|
🎉 This issue has been resolved in version 1.0.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
When using the appmap along with devise authenticated routes such as this snippet from
routes.rb:Attempting to visit an authenticated route yields this exception, regardless of whether the user is authenticated or not. Removing the appmap gem fixes the issue.
The text was updated successfully, but these errors were encountered: