feature: improved logging / error handling ( alternate strategy ) #1904
Conversation
…ten to. ( pulled package-lock.json )
…ten to. -> fixed import from csp
packages/alpinejs/src/utils/error.js
Outdated
|
|
||
| console.warn(`Alpine Expression Error: ${error.message}\n\n${ expression ? 'Expression: \"' + expression + '\"\n\n' : '' }`, el) | ||
|
|
||
| dispatch( document, "alpine:error", { error } ) |
There was a problem hiding this comment.
We probably want to pass the element and expression in here (if they're set)
There was a problem hiding this comment.
Okay, we can add that. They were getting assigned to the error but I agree
There was a problem hiding this comment.
We updated it @HugoDF to be off error object and on main detail object if you want to look again
|
I developed on this branch all day and decided to add the error back. There were a lot of cases where I was not seeing error fast enough. This branch now dispatches the event (which I like for being able to log it or for dev tools) and it throws the error without the downstream side effects we were having before. Can you give it a try @HugoDF? |
|
For the two failing tests they seem to have legitimate errors as far as I can tell, that surface after these changes. For example in store.spec increment() is not defined, the test passed before because the event listener in the alpine-init code handles the click where the @click directive would not have worked. |
|
Any update with getting this merged? I've been using it for development and it's extremely useful. |
|
I've also found some production benefits to this branch. For example, we had google charts api fail when loaded by alpine (just due to CND network) and it would cause all of alpine to lock up but on this branch unrelated components continue to function. Also had some unescaped code causing a component to fail that was locking up alpine and causing listing dropdowns and all kinds of stuff not to work, but on this branch that component fails and is logged and alpine continues to work. So there have been a few unexpected things that this branch solves for us that is especially important in prod. |
|
I'm also eagerly awaiting for this to be merged |
|
I fixed the store.spec.js and customs-directives.spec.js tests that were failing on this branch once error handling was implemented. If there are any questions or feedback that could get this merged I'd really appreciate it. We use this branch in production because it is more fault tolerant (prevents failing component from easily locking up Alpine which we experienced in our app a few times). This has proven to have production and development benefits for our team. The actual error handling strategy (event and re-throw) is isolated to one spot so if @HugoDF or @calebporzio prefer one strategy instead of both I'm indifferent to that part of it. We were just trying to make it easy for dev tools to support. |
|
I haven't had time to look at integrating this but if we've got events that contain the expression and element I'm sure it'll work. |
|
Okay thanks @HugoDF. Like I said we've been using it in production now for few weeks (can view it at www.borderconnect.com) and it's been extremely useful. We started logging client JS errors and it's amazing how much stuff we caught and were able to easily trace with the help of element and expression that we would've been blind to before. Most were extreme edge cases like query from db not having distinct on query and breaking component in very rare cases so it's been great for us. |
|
Thanks for wanting to tackle this. However this PR is stale and the conversation has many different points in it. If you still want this, please re-open with a thorough summary. Thanks again! |
This is an alternative implementation off the previous PR ( #1607 ). One or the other should be chosen, but not both.
Incorporates suggestions from @HugoDF regarding dispatching an event.
We found the dev experience of this version is better as it doesn't cause false down-stream errors. ( other PR causes additional errors when async evaluators error and try to rethrow )