-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Bad Request: Request Line is too large (5465 > 4094) #58
Comments
👍, concerns me too even though I haven't reached the limit yet. Have to support IE7 and higher. |
I would highly highly encourage modifying these values in Apache:
I'm aware that there are limitations across browsers, and I try my best to keep payloads as small as possible, and will be doing things in the future to help it even more. For now though, a POST request provides it's own set of incompatibilities, especially in regards to IE8 and 9, so it's a lose lose situation, honestly. This is all terrible, and it has been a battle over what a browser will even allow us to do. |
@rassie, at this point, raven-js wouldn't even work in IE7 without an external JSON library like |
@mattrobenolt |
So not using POST is CORS-related and -limited, while GET is limited by the URL length (either client-side or server-side)? |
Awesome. I'll be honest, I've done 0 testing at this point for IE7, so I can't guarantee that it works. I've tested with IE8+. |
I haven't upgraded my main Sentry instance to >=5.1.5 yet, but yeah, if it won't work, you'll know. I'm putting great hopes in TraceKit for IE7 ;-) |
Correct. CORS is a much bigger problem, in my opinion, and more tricky to set up/configure/deploy correctly. It's easier to attempt to keep the GET request smaller, and more browsers handle longer querystrings. I will be documenting all of these tips and tricks. We've added some new features to Sentry as well that allows Sentry to fetch the actual lines of context for the error for you, so the client doesn't need to send them. Of course, there's no way for raven-js to automatically know if your assets are at a publicly accessible URL or not without telling it, so we'll probably expose that as an option in the future. This would drastically shorten the payload needed to be sent. Hence why I'd prefer to move forward with a GET instead of dealing with a POST. Bottom line: a GET request is an issue with potential solutions. POST is just a road block that we can't get around. |
@mattrobenolt If Sentry accepts POST payload, could you still introduce an option for those of us who could get their CORS to work, e.g. running Sentry on the same domain? Maybe even with GET as a fallback, but I still consider POSTing a bit cleaner. Thanks. |
It requires a bit of extra bloat and testing to add to the library to support that. If anything, I'd provide two separate builds of raven.js. One that uses POST and one that uses a GET. I'll definitely look into it. At the moment, GET is the most sane way of handling everything. It's limitations aren't that severe. Like I said, Sentry has the features now to fetch js sources on it's own, so the client technically doesn't even need to send as large of a payload if the sources are accessible. It already opts out of sending if the Javascript source is minified, so this problem probably isn't as large as it seems. |
Thanks for the responses. I understand both POST and GET have problems. I tested the current source with a simple error (small stacktrace) and noticed it already didn't work. I'll watch the repo and will test new solutions as they come along. @mattrobenolt I had already tried to change the Apache LimitRequestLine in the first vhost of the bound port, but I still ran into the error. Maybe the wsgi-server has limits too? |
Do you mind sending me an example of a payload generated? I'm curious why Sent from my iPhone On Jan 23, 2013, at 4:42, Ivor notifications@github.com wrote: Thanks for the responses. I understand both POST and GET have problems. I @mattrobenolt https://github.com/mattrobenolt I had already tried to — |
@mattrobenolt This failing image:
This decodes into:
|
Thanks @ivorbosloper. I'll use this as a good reference to see where we can cut corners and reduce the amount of characters being sent over the wire. |
On good news, it captured the error correctly! 👍 |
@ivorbosloper, we're going to, by default, not send the lines of context, which is the majority of the bloat. Since Sentry now supports fetching the source itself, we can avoid all of that and leave it behind a configuration option to enable specifically for scenarios where your app is running behind a firewall or in some other non-publicly accessible location. |
@mattrobenolt I could test a new version and close the issue. Great project btw :) |
Feel free to try out: http://d3nslu0hdya83q.cloudfront.net/build/master/raven.min.js That's our build from master. It's updated as a post-commit hook. This build by default doesn't make an attempt to send the lines of context, but let's Sentry try and fetch them for us. You can disable this behavior, which will make the payload large again, by using Raven.config(..., {fetchContext: true}).install(); |
I have an app using emberjs and some jQuery widgets (like jQuery File Upload widget). The hierarchy is short and quite often I run out of limit: http://wklej.org/id/955778/ And that's even with that raven.min.js. |
@riklaunim, so it looks like everything is correct on our end, just the generated payload is too large. There's nothing we can do about this, except suggest fixing your web server to handle it. The alternatives are not worth the amount of effort needed to support them right now. Sorry. :( |
Yes, that what we are going to do - configure nginx and whatever is between it and sentry to let it pass. At least tracebacks will be meaningful 👍 |
nginx by default is a pretty high value, I'm pretty sure larger than 4096 characters. If not, let me know and I'll add info to the docs. |
This specific message is from gunicorn, not nginx/apache. Actual fix discussed at getsentry/sentry#874 |
Correct me if I'm mistaken: I tthink all CORS setups (fill in allowed domains, send HTTP headers) are managed by sentry, and thus it's not at all very hard to get a working CORS setup on modern browsers. On the other hand, the Image.src method is an ugly hack, and it disguises itself as a sane solution by working fine most of the times, especially in small projects and projects created by the people that decided on the Image.src hack, since i assume they know to avoid it. It's brilliant software, and I think it would be more brilliant if it was using CORS with an Image.src hack as fallback for older browsers. |
I agree, it seems like an overly optimistic solution. |
@wejendorp Not sure I understand your point. This is not typically an issue. Sentry fetches context on the server side, and the client doesn't normally fetch it unless you're operating in a scenario where Sentry won't be able to fetch. For example, in an application behind a firewall, or on a mobile phone, behind HTTP basic auth, etc. Or is this what you're referring to? |
I wasn't aware of that, and I'm sorry for not taking the time to understand what was going on. |
I tried the current master (1.0-beta11) to be able to use setUser(). However, events can not be logged anymore because the request line is too long (my sentry installation is deployed behind Apache, which limits requestline lengh to 4096).
The
makerequest
function has been changed from a POST to a GET:With GET requests the data will always be limited (IE8 and IE9 limit it up to 2083 chars), and I think POST should always be used (or at least be an option).
The text was updated successfully, but these errors were encountered: