Foros de discusión

? The right way to contribute a patch

thumbnail
Sergey Ushakov, modificado hace 15 años.

? The right way to contribute a patch

New Member Mensajes: 19 Fecha de incorporación: 3/06/08 Mensajes recientes
Hi everyone,

some time ago I have submitted a patch to Liferay JIRA for LEP-4428 problem. I have done my best to follow the instructions I found, but probably I have missed something, as nothing has happened to the patch and the bug after that...

Is there any special area at some forum to announce new patches being submitted?

Turning to my specific case, I would really appreciate the patch either merged into the project mainstream or commented by someone. The patch deals with the issue that if there is a problem with SMTP server, most of e-mailing errors will be silently ignored and never brought to anybody's attention. Meanwhile a visitor will be happily but vainly waiting for an e-mail that will never arrive...

Regards,
Sergey
thumbnail
Jorge Ferrer, modificado hace 15 años.

RE: ? The right way to contribute a patch

Liferay Legend Mensajes: 2871 Fecha de incorporación: 31/08/06 Mensajes recientes
Hi Sergey,

Thanks for your patch. This is a good place to announce contributions if they are unheard.

I've taken a look at your patch and seems simple and not harmful. I have a couple of questions:
- Why do you remove the differentiation of the SocketException? I don't know why it was being treated differently, but I would be reluctant to change it without knowing why it was done that way first.
- Why do you need to iterate through nested exceptions manuall? Shouldn't the exception object do it by iself?
thumbnail
Sergey Ushakov, modificado hace 15 años.

RE: ? The right way to contribute a patch - SMTP errors

New Member Mensajes: 19 Fecha de incorporación: 3/06/08 Mensajes recientes
Hi Jorge,

thank you for your clarification and for looking into the patch.

Frankly, I tried a lot to guess the initial intention of the code, but without significant success. My current guess is that the code may contain a programming error, so the current behavior is "provide special guidelines when SMTP server is not set up, and ignore any other problems", while the initial intention might be "provide special guidelines when SMTP server is not set up, and let the other problems be handled in a standard way".

IMHO there are no "harmless" messaging exceptions that might be silently ignored. I'm sure any of them indicates a significant problem either with the portal infrastructure or with the visitor. I believe all of them deserve being reported and logged at ERROR level. Maybe further experience will show that some of them (visitor-related) may deserve some other handling too...

I can't say now where I've borrowed the exception chain formatting pattern. But I have a strong feeling that somewhere in Liferay emoticon - maybe not the code itself but the idea of the formatted result. It's sure that the whole exception chain is to be unwound in order to provide meaningful message. And sure, standard printStackTrace() may do the job, but the printout looks frigtening emoticon I believe not only to me, as I have seen many similar "refined" error messages without stacktrace while setting my Liferay up.

Having revised the whole thing again, I can agree (if somebody insists) that I have removed an important feature of special guidance for the portal admin in case the mail server is not set up at all. If yes, I can add this special case back into the code...

And the last - maybe running ahead a little. IMHO any problem with e-mailing deserves handling in a way that would generate an automatic incident ticket for the help desk. Unfortunately I still did not manage to realize if there is a standard help desk engine in Liferay, so any comments of yours are more than welcome...

Regards,
Sergey