-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
fix(middleware): Catch errors when loading a module (#3572) #3573
Conversation
✅ Build karma 380 completed (commit 891635fc48 by @jehon) |
✅ Build karma 2778 completed (commit 891635fc48 by @jehon) |
✅ Build karma 379 completed (commit 891635fc48 by @jehon) |
✅ Build karma 381 completed (commit 2b0c20184c by @jehon) |
✅ Build karma 2779 completed (commit 2b0c20184c by @jehon) |
✅ Build karma 380 completed (commit 2b0c20184c by @jehon) |
✅ Build karma 382 completed (commit 3054fc50b0 by @jehon) |
✅ Build karma 2780 completed (commit 3054fc50b0 by @jehon) |
✅ Build karma 381 completed (commit 3054fc50b0 by @jehon) |
✅ Build karma 2781 completed (commit 1c220fa432 by @jehon) |
✅ Build karma 383 completed (commit 1c220fa432 by @jehon) |
✅ Build karma 382 completed (commit 1c220fa432 by @jehon) |
✅ Build karma 384 completed (commit 5f389087bd by @jehon) |
✅ Build karma 2782 completed (commit 5f389087bd by @jehon) |
✅ Build karma 383 completed (commit 5f389087bd by @jehon) |
@johnjbarton Sorry to interrupt you, but could you have an (updated) opinion on this PR? |
Could someone review this PR? |
Sorry for the long delay. I want to investigate the possibility of correcting any tests that fail with the error on both paths. |
I did look into the failings tests, nothing important. It's just that it test the string, and that in need to be adapted. I can push it here if wanted. But the problem is that the output would change for errors in normal scripts (here, I put a call to "nothing()":
Instead of
There is no difference with the "if". |
Followup in #3585 |
Followup in #3605 |
Fix #3572