Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

request/2 matching _Other leads to 400 #60

Closed
beerriot opened this Issue · 5 comments

4 participants

Bryan Fink David Reid Dmitry Demeshchuk Bob Ippolito
Bryan Fink

I'm curious about the catch-all match in the receive block of mochiweb_http:request/2:

https://github.com/mochi/mochiweb/blob/62e460dbd424c35185636a4b39f0d9c4a3211de2/src/mochiweb_http.erl#L69

Mainly, why does this clause trigger an error, instead of just ignoring the message?

This behavior is causing trouble for me because I have a request handler (a Webmachine resource) that spawns other processes and waits for messages from them. The request handler may decide to send a response to the client before it receives all of the messages from the other processes, though. If Req:should_close() returns false after the handler finishes, the process ends up back in this receive block, with the possibility of some leftover messages ripe for picking from its mailbox. I'd rather just have those messages dropped on the floor, instead of causing errors here.

Could my use case be fixed by possibly expanding the list of clauses to find other types of errors that mochiweb is interested in, while ignoring all other messages, or is there a reason I'm missing for blowing up with a 400 here?

This question will also apply to mochiweb_http:headers/5:

https://github.com/mochi/mochiweb/blob/62e460dbd424c35185636a4b39f0d9c4a3211de2/src/mochiweb_http.erl#L98

As noted, I'm dealing with a Webmachine resource, so I'm actually working with mochiweb 1.5.1, but the clause there is very similar.

Thanks,
Bryan

David Reid
Collaborator

You should use an intermediate process to isolate this message queue from your application code handling the request.

There are too many opportunities for messages to pollute this queue otherwise and mochiweb can't possibly be expected to know when the developer wants that to cause a real error or hang his application. In most cases unhandled messages in this queue are unintended and likely a bug.

David Reid dreid closed this
Dmitry Demeshchuk
Owner

Maybe adding a function like mochiweb_request:close/0 will be a better solution for this? Trying to catch some custom messages or errors seems like a tombstone to me.

Bryan Fink

Thanks for your response, @dreid. I was hoping to have one less process hop for my data to pass through, but I agree that it's better to keep that queue clean.

As an interim fix (just to prevent spurious messages being sent to the client), I'm considering using the undocumented mochiweb_request_force_close process dict entry. Any pitfalls you would warn me about?

Bob Ippolito
Owner

What it's really looking for are unexpected messages from the socket, but @dreid is right in that you probably shouldn't expect this behavior. If mochiweb did handle only messages that a socket could send, you'd have a memory leak (for the duration of the connection anyway).

You'd be better off with an additional process than trying to do undocumented stuff with mochiweb's internals. I guarantee that I will break support for that stuff at some point, we don't use it in any of our code and it's not public API.

Bryan Fink

Thanks, @etrepum. Duly noted.

To be clear, though, by "ignore", I meant, "receive and ignore" (via tail call right back to the receive block), not "leave in the mailbox". I've sometimes found that pattern necessary just to protect against spurious messages from things like pid reuse and debug prodding.

But, yes, still agreed that I should separate these two pieces, and I shall do so before upgrading mochiweb. Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.