Skip to content
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

Disable maxMessageLength truncation by default #441

Merged
merged 2 commits into from
Dec 30, 2015
Merged

Conversation

mattrobenolt
Copy link
Contributor

We can now send lots more data since we send via POST, so remove this restriction.

Alternatively, we can either remove this entirely, or handle a 0 value to disable truncation entirely. I can't see any other client that does this type of truncation. iirc, I added this originally to limit the packet size being sent, which isn't really needed anymore.

@benvinegar
Copy link
Contributor

👍

Should help those Angular messages.

@mattrobenolt
Copy link
Contributor Author

Do you have a preference on longer message length or default to no truncation? I'm not even sure that 1000 would cover all the Angular stuff?

@benvinegar
Copy link
Contributor

Mmm. Yeah, let's remove it ... and if it truly becomes a problem, reinstate it. (I'm guessing it wont.)

@mattrobenolt mattrobenolt changed the title Bump default maxMessageLength to 1000 Disable maxMessageLength truncation by default Dec 30, 2015
@mattrobenolt
Copy link
Contributor Author

@benvinegar Alright, it's just disabled by default now instead. I'd rather leave it in for now in case someone wants to explicitly limit it if they were using their own transport or whatever and had a limitation.

@benvinegar
Copy link
Contributor

Works for me.

mattrobenolt added a commit that referenced this pull request Dec 30, 2015
Disable maxMessageLength truncation by default
@mattrobenolt mattrobenolt merged commit 70f07ed into master Dec 30, 2015
@mattrobenolt mattrobenolt deleted the maxlength branch December 30, 2015 01:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants