jdufresne
Repos
72
Followers
266
Following
32

Events

issue comment
[Bug]: Unhandled exception in the data loader does not print the stack trace to the console

Yes, that is right. The stack renders inside the default errorElement, just not in the console.

Created at 7 hours ago
opened issue
[Bug]: Unhandled exception in the data loader does not print the stack trace to the console

What version of React Router are you using?

6.9.0

Steps to Reproduce

Use the example:

import React from "react";
import {
    createBrowserRouter,
    RouterProvider,
    useLoaderData,
} from "react-router-dom";

let router = createBrowserRouter([
    {
        path: "/",
        loader: homeLoader,
        element: <Home />,
    },
]);

export default function App() {
    return <RouterProvider router={router} />;
}

export async function homeLoader() {
    throw new Error('bug!')
    return {};
}

export function Home() {
    let data = useLoaderData() as HomeLoaderData;
    return <h2>Home</h2>;
}

In this example, there is a bug in homeLoader that throws an unhandled exception.

This exception causes the errorElement to render (good and expected) but the error itself is never printed to the JavaScript console using console.error or similar.

I consider this a bug as the docs say:

https://reactrouter.com/en/main/route/error-element

If you do not provide an errorElement in your route tree to handle a given error, errors will bubble up and be handled by a default errorElement which will print the error message and stack trace. Some folks have questioned why the stack trace shows up in production builds. Normally, you don't want to expose stack traces on your production sites for security reasons. However, this is more applicable to server-side errors (and Remix does indeed strip stack traces from server-side loader/action responses). In the case of client-side react-router-dom applications the code is already available in the browser anyway so any hiding is just security through obscurity. Furthermore, we would still want to expose the error in the console, so removing it from the UI display is still not hiding any information about the stack trace. Not showing it in the UI and not logging it to to the console would mean that application developers have no information at all about production bugs, which poses its own set of issues. So, again we recommend you always add a root level errorElement before deploying your site to production!

I understand this mean the error should be printed to the console. Errors that occur during rendering (instead of in the loader) do print to the console as expected.

Printing the error to the console would improve the developer experience for debugging and testing.

Expected Behavior

The unhandled error in the loader is printed to the JavaScript console with its stack trace.

Actual Behavior

The unhandled error does not appear not JavaScript console.

The errorComponent does render.

Created at 8 hours ago

Merge branch 'release-next' into dev

Add Layout Route Example (#10011)

Co-authored-by: Tim Dorr timdorr@users.noreply.github.com

chore: format

docs(start/tutorial): fix EditContact component typo (#10015)

chore: sort contributors list

docs(hooks/use-matches): Fix crumb route (#10020)

chore: sort contributors list

Added better detection for absolute urls in link component (#9994)

docs(start/tutorial) fix: highlighting for changing <form> to <Form> (#10025)

Co-authored-by: Mehdi Achour machour@gmail.com

docs(components/form): Fix component name (#10018)

chore: sort contributors list

chore(deps): bump examples to latest (#10026)

Signed-off-by: Logan McAnsh logan@mcan.sh

Only check for differing origin on absolute URL redirects (#10033)

ci: use current branch when gathering PRs for stable releases (#10032)

sync with remix https://github.com/remix-run/remix/pull/5342

Signed-off-by: Logan McAnsh logan@mcan.sh

ci: use current branch when gathering PRs for stable releases (#10032)

sync with remix https://github.com/remix-run/remix/pull/5342

Signed-off-by: Logan McAnsh logan@mcan.sh (cherry picked from commit 6120207a328740a226c462d87be479b7c26b9574)

Fix partial object (search or hash only) pathnames losing current path (#10029)

Co-authored-by: Logan McAnsh logan@remix.run

Remove inaccurate console warning for POP navigations (#10030)

Merge branch 'main' into release-next

Enter prerelease mode

chore: Update version for release (pre) (#10035)

Created at 10 hours ago

Install typescript-mode

Created at 1 day ago

Update README to use new gem name for install script

Created at 1 day ago

Make the test environment show rescuable exceptions in responses

Background

During integration tests, it is desirable for the application to respond as closely as possible to the way it would in production. This improves confidence that the application behavior acts as it should.

In Rails tests, one major mismatch between the test and production environments is that exceptions raised during an HTTP request (e.g. ActiveRecord::RecordNotFound) are re-raised within the test rather than rescued and then converted to a 404 response.

Setting config.action_dispatch.show_exceptions to true will make the test environment act like production, however, when an unexpected internal server error occurs, the test will be left with a opaque 500 response rather than presenting a useful stack trace. This makes debugging more difficult.

This leaves the developer with choosing between higher quality integration tests or an improved debugging experience on a failure.

I propose that we can achieve both.

Solution

Change the configuration option config.action_dispatch.show_exceptions from a boolean to one of 3 values: :all, :rescuable, :none. The values :all and :none behaves the same as the previous true and false respectively. What was previously true (now :all) continues to be the default for non-test environments.

The new :rescuable value is the new default for the test environment. It will show exceptions in the response only for rescuable exceptions as defined by ActionDispatch::ExceptionWrapper.rescue_responses. In the event of an unexpected internal server error, the exception that caused the error will still be raised within the test so as to provide a useful stack trace and a good debugging experience.

Created at 1 day ago

Make the test environment show rescuable exceptions in responses

Background

During integration tests, it is desirable for the application to respond as closely as possible to the way it would in production. This improves confidence that the application behavior acts as it should.

In Rails tests, one major mismatch between the test and production environments is that exceptions raised during an HTTP request (e.g. ActiveRecord::RecordNotFound) are re-raised within the test rather than rescued and then converted to a 404 response.

Setting config.action_dispatch.show_exceptions to true will make the test environment act like production, however, when an unexpected internal server error occurs, the test will be left with a opaque 500 response rather than presenting a useful stack trace. This makes debugging more difficult.

This leaves the developer with choosing between higher quality integration tests or an improved debugging experience on a failure.

I propose that we can achieve both.

Solution

Change the configuration option config.action_dispatch.show_exceptions from a boolean to one of 3 values: :all, :rescuable, :none. The values :all and :none behaves the same as the previous true and false respectively. What was previously true (now :all) continues to be the default for non-test environments.

The new :rescuable value is the new default for the test environment. It will show exceptions in the response only for rescuable exceptions as defined by ActionDispatch::ExceptionWrapper.rescue_responses. In the event of an unexpected internal server error, the exception that caused the error will still be raised within the test so as to provide a useful stack trace and a good debugging experience.

Created at 1 day ago

Allow destroying active storage variant

Background:

When creating active storage variants, ActiveStorage::VariantRecord is inserted, then a file is uploaded. Because upload can be failed, the file can be missing even though ActiveStorage::VariantRecord exists.

When a file is missing, we need to delete the corresponding ActiveStorage::VariantRecord but there's no API to delete just one variant e.g., blob.variant(resize_to_limit: [100, 100]).destroy.

Co-authored-by: Yuichiro NAKAGAWA ii.hsif.drows@gmail.com Co-authored-by: Ryohei UEDA ueda@anipos.co.jp

Allow Attachables to override default template when attachment is missing

Adds documentation for Arel::Nodes::Node

Support prerelease rubies in Gemfile template if possible

This commit brings back #45979 only when RubyGems version is 3.3.13 or higher that includes https://github.com/rubygems/rubygems/pull/5486

Pull request #45979 has been reverted via #47524 because the default versions of RubyGems shipped with Ruby 2.7 and 3.0 are lower than 3.3.13. If users who bumps the RubyGems version or uses Ruby 3.1 or higher, RubyGems version should be satisfied to support prerelease rubies.

Ref #45979 #47524 Fix #47542

Co-authored-by: David Rodríguez deivid.rodriguez@riseup.net

Look for created.rid in api_dir (output dir)

This changes to use a single source of truth for the output dir in the rdoc task. Which lets us override this value when building from a different directory.

Mainly this change is a no-op, but we are using Rails::API:Task to test and build SDoc version 3.

Quote binary strings for SQLite3 and MySQL

We should properly hex-encode binary strings for SQLite3 and MySQL. For e.g. MySQL, not doing that may work, but it will also produce database warnings.

Ensure BigDecimals are not binary-encoded

In Ruby 2.7, BigDecimal#to_s generates ASCII-8BIT-encoded strings. We don't want those to get hex-encoded in the generated SQL.

Merge pull request #47590 from zzak/rdoc-task-output-dir

Look for created.rid in api_dir (output dir)

Lockdown rails app in production for security (#47594)

Current Dockerfile generated by Rails runs as a non-root user which prevents modification of the operating system but leaves wide open all gems and the application itself.

This change locks down the application gems and only opens up access to the following directories: db, log, storage, tmp

This is a even more secure alternative to

https://github.com/rails/rails/pull/47580

Make some database tasks methods private

These were never supposed to be public and they aren't very useful outside of database tasks so make them private.

Merge pull request #47605 from eileencodes/make-some-db-task-methods-private

Make some database tasks methods private

Fix #47535 - flag multiple cookies as secure

Fix schema cache file test

This test was leaving behind the directory and was never verifying the file existed after dump was called. We can't use a tempfile because it will be created automatically and this is testing that SchemaCache#open will create the file if it doesn't exist.

Merge pull request #47606 from eileencodes/fix-schema-cache-file-test

Fix schema cache file test

Factor out valid_column_definition_options

Shopify is implementing a custom ActiveRecord adapter to integrate with Vitess, and we would like to overload the valid_{column,table}_definition_options methods and add additional valid options for schema migrations.

For example:

module ActiveRecord
  module ConnectionAdapters
    class VitessMysql2Adapter < Mysql2Adapter
      ...

      def valid_table_definition_options
        super + [:skip_vschema_migrations, :sharding_keys, :auto_increment]
      end

      def valid_column_definition_options
        super + [:skip_vschema_migrations, :sharding_keys, :auto_increment]
      end
    end
  end
end

This is the simplest possible change and factors out the various valid_{table,column,primary_key}_definition_options to be a public method on an adapter instance.

Fix rails new --dev APP_PATH command crashing

ref: https://github.com/rails/rails/issues/47435#issuecomment-1436530001

  • rails new APP_PATH --dev works fine.
  • rails new --dev APP_PATH does not work.

For most other flags, putting the APP_PATH before or after the flags has the same effect. It's just the prelease ones that have issues, because we have to rengerate the command so as to call bundle exec rails new ... a second time.

Prior to this PR, the command that was being called was bundle exec rails new APP_PATH APP_PATH, and that doesn't work. Now it will get called as bundle exec rails new APP_PATH --dev APP_PATH which is fine.

I haven't been able to find a way to write an automated test for this, but you can use the replication steps in https://github.com/rails/rails/issues/47435 to verify that the issue is fixed.

Co-authored-by: Jonathan Hefner jonathan@hefner.pro

Move SQLite3 blob encoding to ActiveModel

Merge pull request #47493 from olefriis/quote_binary_strings

Quote binary strings in Arel

Use an actual version in .node-version

"lts" isn't supported by all version managers, and even if it was, it's a moving target, so it might build differently on different machines based on how the version manager refresh it's definition list.

Merge pull request #47461 from Shopify/docker-node-latest-lts

Use an actual version in .node-version

Created at 2 days ago
create tag
jdufresne create tag v6.1.7.3
Created at 2 days ago
create tag
jdufresne create tag v7.0.4.3
Created at 2 days ago

Allow destroying active storage variant

Background:

When creating active storage variants, ActiveStorage::VariantRecord is inserted, then a file is uploaded. Because upload can be failed, the file can be missing even though ActiveStorage::VariantRecord exists.

When a file is missing, we need to delete the corresponding ActiveStorage::VariantRecord but there's no API to delete just one variant e.g., blob.variant(resize_to_limit: [100, 100]).destroy.

Co-authored-by: Yuichiro NAKAGAWA ii.hsif.drows@gmail.com Co-authored-by: Ryohei UEDA ueda@anipos.co.jp

Allow Attachables to override default template when attachment is missing

Adds documentation for Arel::Nodes::Node

Support prerelease rubies in Gemfile template if possible

This commit brings back #45979 only when RubyGems version is 3.3.13 or higher that includes https://github.com/rubygems/rubygems/pull/5486

Pull request #45979 has been reverted via #47524 because the default versions of RubyGems shipped with Ruby 2.7 and 3.0 are lower than 3.3.13. If users who bumps the RubyGems version or uses Ruby 3.1 or higher, RubyGems version should be satisfied to support prerelease rubies.

Ref #45979 #47524 Fix #47542

Co-authored-by: David Rodríguez deivid.rodriguez@riseup.net

Look for created.rid in api_dir (output dir)

This changes to use a single source of truth for the output dir in the rdoc task. Which lets us override this value when building from a different directory.

Mainly this change is a no-op, but we are using Rails::API:Task to test and build SDoc version 3.

Quote binary strings for SQLite3 and MySQL

We should properly hex-encode binary strings for SQLite3 and MySQL. For e.g. MySQL, not doing that may work, but it will also produce database warnings.

Ensure BigDecimals are not binary-encoded

In Ruby 2.7, BigDecimal#to_s generates ASCII-8BIT-encoded strings. We don't want those to get hex-encoded in the generated SQL.

Merge pull request #47590 from zzak/rdoc-task-output-dir

Look for created.rid in api_dir (output dir)

Lockdown rails app in production for security (#47594)

Current Dockerfile generated by Rails runs as a non-root user which prevents modification of the operating system but leaves wide open all gems and the application itself.

This change locks down the application gems and only opens up access to the following directories: db, log, storage, tmp

This is a even more secure alternative to

https://github.com/rails/rails/pull/47580

Make some database tasks methods private

These were never supposed to be public and they aren't very useful outside of database tasks so make them private.

Merge pull request #47605 from eileencodes/make-some-db-task-methods-private

Make some database tasks methods private

Fix #47535 - flag multiple cookies as secure

Fix schema cache file test

This test was leaving behind the directory and was never verifying the file existed after dump was called. We can't use a tempfile because it will be created automatically and this is testing that SchemaCache#open will create the file if it doesn't exist.

Merge pull request #47606 from eileencodes/fix-schema-cache-file-test

Fix schema cache file test

Factor out valid_column_definition_options

Shopify is implementing a custom ActiveRecord adapter to integrate with Vitess, and we would like to overload the valid_{column,table}_definition_options methods and add additional valid options for schema migrations.

For example:

module ActiveRecord
  module ConnectionAdapters
    class VitessMysql2Adapter < Mysql2Adapter
      ...

      def valid_table_definition_options
        super + [:skip_vschema_migrations, :sharding_keys, :auto_increment]
      end

      def valid_column_definition_options
        super + [:skip_vschema_migrations, :sharding_keys, :auto_increment]
      end
    end
  end
end

This is the simplest possible change and factors out the various valid_{table,column,primary_key}_definition_options to be a public method on an adapter instance.

Fix rails new --dev APP_PATH command crashing

ref: https://github.com/rails/rails/issues/47435#issuecomment-1436530001

  • rails new APP_PATH --dev works fine.
  • rails new --dev APP_PATH does not work.

For most other flags, putting the APP_PATH before or after the flags has the same effect. It's just the prelease ones that have issues, because we have to rengerate the command so as to call bundle exec rails new ... a second time.

Prior to this PR, the command that was being called was bundle exec rails new APP_PATH APP_PATH, and that doesn't work. Now it will get called as bundle exec rails new APP_PATH --dev APP_PATH which is fine.

I haven't been able to find a way to write an automated test for this, but you can use the replication steps in https://github.com/rails/rails/issues/47435 to verify that the issue is fixed.

Co-authored-by: Jonathan Hefner jonathan@hefner.pro

Move SQLite3 blob encoding to ActiveModel

Merge pull request #47493 from olefriis/quote_binary_strings

Quote binary strings in Arel

Use an actual version in .node-version

"lts" isn't supported by all version managers, and even if it was, it's a moving target, so it might build differently on different machines based on how the version manager refresh it's definition list.

Merge pull request #47461 from Shopify/docker-node-latest-lts

Use an actual version in .node-version

Created at 2 days ago
delete branch
jdufresne delete branch typos
Created at 1 week ago
delete branch
jdufresne delete branch typography
Created at 2 weeks ago
issue comment
Improve typography of user facing validation messages

Something like the following may assist with a one time global search and replace:

find . -name \*.rb -exec sed -i -e "s/can't be blank/can’t be blank/g" {} \;

Adjust the regex for the HTML escaped form too.


I also agree it's a pain to type

FWIW, I use emacs and it provides a key binding (C-x 8 RET) for entering Unicode code points that do not exist on the keyboard. I personally use this for a lot more than just U+2019. Your favorite editor may have a similar feature.

Created at 2 weeks ago

Change dump property response.status_text to response.statusText

Match the JavaScript Response constructor.

https://developer.mozilla.org/en-US/docs/Web/API/Response/Response

Created at 2 weeks ago
delete branch
jdufresne delete branch fix-reconfigure
Created at 2 weeks ago

[office-js][office-js-preview] (PowerPoint) Updating TagCollection.getItem description (#64623)

🤖 Merge PR #64625 Update chrome.fileSystemProvider in chrome/index.d.ts by @lucmult

  • Update chrome.fileSystemProvider

  • reformat and fix doc s/is_directory/name

  • Update EntryMetadata field docs


Co-authored-by: Luciano Pacheco lucmult@chromium.org

[README]: Improve Chinese translation (#64116)

  • [README]: rename zh.md to zh-Hans.md

Using zh is ambiguous because it does not tell it is Simplified Chinese or Traditional Chinese. As this translation is actually in Simplified Chinese, so rename it.

  • [README]: improve Simplified Chinese translation

  • [README]: adopt translation from current zh docs

PR #63899 updated the Chinese translation and was merged. I took a look at their work and adopted some of their tranlations that are better than mine.

  • [README]: Update links in es.md and pt.md

Also change title of link to zh-Hans.md from "Chinese" to "Simplified Chinese".

  • [README]: Sync with latest English readme

Update ramda test for TS bugfix (#64666)

🤖 Merge PR #64657 Update TokenClientConfig in google.accounts by @me-ZaidAli

  • Update TokenClientConfig in google.accounts

Updated the scope prop from being optional to being required ad mentioned in docs

  • Update google.accounts-tests.ts

Nightwatch: delete unused jsdoc types (#64642)

Previously, the lint rule forbidding types in jsdoc was crashing. Now that it's not, it found some types in nightwatch's jsdoc. After the types are gone, there's not really any additional text in the param tags, so I deleted them, too. The whole thing is redundant with the type annotations.

Update dom-screen-wake-lock for TS 5.1 (#64671)

Typescript 5.1's DOM types add the same types as dom-screen-wake-lock. This PR updates the types to make them agree so that people can keep installing (the latest version of) this package.

However, I don't think this package will actually be needed after 5.1, because all of its types are now in the standard library.

Update dom-webcodecs for TS 5.1 (#64672)

  • Update dom-webcodecs for TS 5.1

Typescript 5.1's DOM types add some more of the types as dom-webcodecs. This PR updates the types to make them agree.

  • update minimum TS version of dependent

Update web-animations-js for TS 5.1 (#64674)

Typescript 5.1's DOM types add some more of the types of web-animations-js. This PR updates the types to make them agree and removes the duplicate type declarations.

Update w3c-css-typed-object-model-level-1 for TS 5.1 (#64673)

Typescript 5.1's DOM types add some more of the types as w3c-css-typed-object-model-level-1. This PR updates the types to make them agree and removes the duplicate type declarations.

As of TS 5.1, I think the package's types are mostly redundant, but existing dependents will be able to update, and they might want to continue to use any non-standard types.

Update webappsec-credential-management for TS 5.1 (#64676)

Typescript 5.1's DOM types add some more of the types of webappsec-credential-management. This PR updates the types to make them agree and removes the duplicate type declarations.

Created at 2 weeks ago
Change dump property response.status_text to response.statusText

Match the JavaScript Response constructor.

https://developer.mozilla.org/en-US/docs/Web/API/Response/Response

Created at 2 weeks ago

Change dump property response.status_text to response.statusText

Match the JavaScript Response constructor.

https://developer.mozilla.org/en-US/docs/Web/API/Response/Response

Created at 2 weeks ago
jdufresne delete branch camel-case
Created at 2 weeks ago
Change dump property response.status_text to response.statusText

Match the JavaScript Response constructor.

https://developer.mozilla.org/en-US/docs/Web/API/Response/Response

Created at 2 weeks ago
jdufresne create branch camel-case
Created at 2 weeks ago

Add CLI option to exclude response headers (#68)

Add CLI option to exclude response headers.

Response headers could make response dumper output nondeterministic. To help with test environments that do not require response headers the CLI option can now be used to filter out the extra content.

The snapshot test helper was updated to allow for optionally specifying a non standard snapshot directory to help test both including and excluding response headers on the test app.

Created at 2 weeks ago