feat(errorHandlingConfig): add an option not to encode stack in URL #16283
Conversation
error is truncated
In Angular.js the error in loading a module will lead to a failur in loading
the dependent modules. In turn, this leads to a chain of errors in loading
those modules. For some reason, angularjs is copying the error stack and
passes it to the next error. This stack is encoded in URL error,so we end up
sometimes with so long Error URL when we have may dependent modules.
This option disables encoding the parameters in URL error:
angular.errorHandlingConfig({isUrlParameter:false});
closes angular#14744
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
1 similar comment
|
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
|
I'm pretty sure I have signed a CLA for our company and @aalmkhieber is one of our employees. I'm not quite sure how to resolve the CLA issue. Please advise. |
|
As discussed, it's generally nice to have at least some context, and a bit of stack can definitely help. So what about an option to set the stack length (not the url length)? Then you can set it to 0 when you really don't want it, but provide a sensible default to show some context at least. |
aalmkhieber
changed the title
I am wrong here, |
|
@frederikprijck, n that is not correct. Company CLAs are a thing 😃 @IgorMinar, can you confirm/verify the CLA? |
|
@oliversalzburg @gkalpak Okay, ignore that part. Made (still makes) little sense to me if the commit is done using a single github account. But nvm I guess 🙈 |
|
@frederikprijck It's not done using a single account. I have my own account and so does every other employee. We have signed a corporate CLA with Google, so that all our employees are allowed to contribute to AngularJS. I neglected to add @aalmkhieber to the Google Group associated with our CLA, but I have done so in the meanwhile. At least, I think I did, as the process is not as clear as I would like it to be :D If someone could offer some feedback on the implementation, that would be awesome. |
I did, here: #16283 (comment) |
|
I've left more info in the first thread: #15707 (comment) |
|
@aalmkhieber actually had more extensive configuration options available in a PoC. He's currently researching how to properly implement them. @aalmkhieber Could you update us on the state of development? If there are any open questions, I'm sure someone can offer some guidance. |
|
I suggest that we can add two more options to error configuration to control the behaviour of loadModules function in terms of catched errors:
Implementing isModuleErr through just rethrowing the catched error will cause the stack to be changed in chrome: Please advise! |
I couldn't reproduce it and that issue is closed as fixed 😕 |
|
I don't think adding even more config options is very developer friendly. |
|
@Narretz I don't get it. I complain about the error message and I'm being told "Hey, we're adding configuration capabilities. That's the way to solve this problem." Now we're adding configuration options to that capability and suddenly it's not the way to go? |
|
@oliversalzburg This is my opinion and other members might disagree, however I feel like we should discuss the options, but instead I didn't get any clear response to the points I made, which I find rather strange tbh, especially if the answer is simply "more code". |
|
@Narretz I agree that the second and the third options are too specific but they offer us a solution to these too long messages. And I think the output of the script: |
|
I will look into how my proposal affects other error messages, and put it up for discussion at the next team meeting. I guess an extra option to turn off all arguments doesn't hurt, either. |
|
@Narretz Great! |
|
We've put this on the back burner as we are trying to get up to speed with the 1.7 release, but I think in general we'll implement it with an option to restrict the args length (so you can set it to 0, and get the plain error url) |
|
... |
Specific errors, such as nested module loading, can create very long error urls because they include the error stack. These urls create visual clutter in the browser console, are often not clickable, and may be rejected by the docs page because they are simply too long. We've already made improvements to the error display in angular#16283, which excludes the error url from error parameters, which results in cleaner error messages. Further, modern browsers restrict console message length intelligently. This option can still be useful for older browsers like Internet Explorer, or in general to reduce visual clutter in the console. Closes angular#14744
Specific errors, such as nested module loading, can create very long error urls because they include the error stack. These urls create visual clutter in the browser console, are often not clickable, and may be rejected by the docs page because they are simply too long. We've already made improvements to the error display in angular#16283, which excludes the error url from error parameters, which results in cleaner error messages. Further, modern browsers restrict console message length intelligently. This option can still be useful for older browsers like Internet Explorer, or in general to reduce visual clutter in the console. Closes angular#14744 Closes angular#15707 Closes angular#16283 Closes angular#16299
Specific errors, such as those during nested module loading, can create very long error urls because the error message includes the error stack. These urls create visual clutter in the browser console, are often not clickable, and may be rejected by the docs page because they are simply too long. We've already made improvements to the error display in #16283, which excludes the error url from error parameters, which results in cleaner error messages. Further, modern browsers restrict console message length intelligently. This option can still be useful for older browsers like Internet Explorer, or in general to reduce visual clutter in the console. Closes #14744 Closes #15707 Closes #16283 Closes #16299 Closes #16591
Specific errors, such as those during nested module loading, can create very long error urls because the error message includes the error stack. These urls create visual clutter in the browser console, are often not clickable, and may be rejected by the docs page because they are simply too long. We've already made improvements to the error display in #16283, which excludes the error url from error parameters, which results in cleaner error messages. Further, modern browsers restrict console message length intelligently. This option can still be useful for older browsers like Internet Explorer, or in general to reduce visual clutter in the console. Closes #14744 Closes #15707 Closes #16283 Closes #16299 Closes #16591
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Feature
What is the current behavior? (You can also link to an open issue here)
When there is an error in loading a module then all modules that are dependent will fail also in
loading. The throwed error contains the URL for each error. The URL contains encoded stack which leads sometimes to long URL.
What is the new behavior (if this is a feature change)?
Adding an option so that the generated URL contains only the erorr code which is informative enough
to find out the problem. closes #14744.
Does this PR introduce a breaking change?
Please check if the PR fulfills these requirements
Other information: