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

Component renders "" (blank) when an error is raised during the render phase #1981

Closed
mjacobus opened this issue Jan 23, 2024 · 10 comments · Fixed by newrelic/newrelic-ruby-agent#2410

Comments

@mjacobus
Copy link

mjacobus commented Jan 23, 2024

Steps to reproduce

Render a component from a controller

class MyBrokenComponent < ViewComponent::Base
  erb_template <<-ERB
    foo <% raise_an_error %> bar
  ERB

  def raise_an_error
    raise "an error"
  end
end

class MyController < ActionController::Base
  def show
    render MyBrokenComponent.new
  end
end

Expected behavior

I expected to see the rails error page.

Actual behavior

A blank page.

A few things worth noting

  1. If just one of the components raises an error, only that component will be blank. The rest of the page will work:
<!-- my_page_component.html.erb -->
<%= render SpanComponent.new("one") %>
<%= render MyBrokenComponent.new %>
<%= render SpanComponent.new("two") %>

The page will render:

<span>one</span>
<span>two</span>
  1. If MyBrokenComponent raises during in the initializer, the page will be blank.
<!-- my_page_component.html.erb -->
<%= render SpanComponent.new("one") %>
<%= render MyBrokenComponent.new %>
<%= render SpanComponent.new("two") %>
  1. If the component rendered by the controller raises an error in the initializer, the expected error page will be displayed.
class MyBrokenComponent < ViewComponent::Base
  erb_template <<-ERB
    foo bar
  ERB

  def initialize
    raise "an error"
  end
end

class MyController < ActionController::Base
  def show
    # the rails error page will be rendered
    render MyBrokenComponent.new 
  end
end

So all of that, makes me believe that the error suppressing is happening during the rendering phase.

Backtrace:

System configuration

Rails version: Rails 7.0.8
Ruby version: ruby 3.0.6p216 (2023-03-30 revision 23a532679b) [x86_64-darwin21]
Gem version: view_component (3.10.0), view_component (3.5.0), view_component (3.0.0) (without the erb_template syntax.

@mjacobus mjacobus changed the title Component renders "" (blank) when an error is raised Component renders "" (blank) when an error is raised during the render phase Jan 23, 2024
@reeganviljoen
Copy link
Collaborator

@mjacobus if you are able to add failing tests in a pr it would help a lot, otherwise someone will be able to do it

@mjacobus
Copy link
Author

Update:

I downgraded view_component to 2.80.0 and the error was still there. So I think it is something in our application 🤔

@mjacobus
Copy link
Author

@reeganviljoen noted! 👍🏻 I will dig some more, and I come back to it.

@reeganviljoen
Copy link
Collaborator

@mjacobus i also use new relic + view_component I will issue a bug report if you haven't already

@mjacobus
Copy link
Author

Thank you @reeganviljoen.

This is the PR that introduced this behavior: https://github.com/newrelic/newrelic-ruby-agent/pull/2367/files#r1463895980

mjacobus added a commit to mjacobus/newrelic-ruby-agent that referenced this issue Jan 23, 2024
We should be able to se error pages, specially in when Rails.env is
development. The piece of code that was removed here
prevented from happening. Instead, it would have the `render_in` method
return `nil` when an error happened, resulting in either a blank page,
or an incomplete page.

I believe we also want to raise errors in production. We should not make
a decision here, whether the error should be suppressed. Otherwise we
are making the assumption "incomplete pages are better than error
pages", which may be the correct assumption under some circumstances,
but not under other circumstances.

Fixes ViewComponent/view_component#1981
@mjacobus
Copy link
Author

Here's a PR that should fix the issue: newrelic/newrelic-ruby-agent#2410

mjacobus added a commit to mjacobus/newrelic-ruby-agent that referenced this issue Jan 23, 2024
We should be able to se error pages, specially in when Rails.env is
development. The piece of code that was removed here
prevented from happening. Instead, it would have the `render_in` method
return `nil` when an error happened, resulting in either a blank page,
or an incomplete page.

I believe we also want to raise errors in production. We should not make
a decision here, whether the error should be suppressed. Otherwise we
are making the assumption "incomplete pages are better than error
pages", which may be the correct assumption under some circumstances,
but not under other circumstances.

Fixes ViewComponent/view_component#1981
@reeganviljoen
Copy link
Collaborator

@mjacobus what a legend, can't wait for them to fix it 🙏

@mjacobus
Copy link
Author

For the record, we can disable view component instrumentation with the instrumentation.view_component option.

# config/newrelic.yml
common: &default_settings
  # [...]
  instrumentation.view_component: disabled

mjacobus added a commit to mjacobus/newrelic-ruby-agent that referenced this issue Jan 24, 2024
We should be able to se error pages, specially in when Rails.env is
development. The piece of code that was removed here
prevented from happening. Instead, it would have the `render_in` method
return `nil` when an error happened, resulting in either a blank page,
or an incomplete page.

I believe we also want to raise errors in production. We should not make
a decision here, whether the error should be suppressed. Otherwise we
are making the assumption "incomplete pages are better than error
pages", which may be the correct assumption under some circumstances,
but not under other circumstances.

Fixes ViewComponent/view_component#1981
@kinduff
Copy link

kinduff commented Jan 24, 2024

@mjacobus You are a legend, thank you so much!

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

Successfully merging a pull request may close this issue.

3 participants