Re: [jsp-dev] Discussion on deprecating the ErrorData constructor withou

[Paul Nicolucci <pnicolucci@xxxxxxxxx>]

I just noticed ErrorData is a final Class: https://github.com/jakartaee/pages/blob/master/api/src/main/java/jakarta/servlet/jsp/ErrorData.java#L29.

My initial thinking was if someone was extending ErrorData and overriding PageContext.getErrorData() but that doesn't look to be a valid concern.

So I think the only argument against the deprecation and eventual removal would be if an application/container were using the old Constructor of ErrorData.

Regards,

Paul Nicolucci

On Wed, Oct 25, 2023 at 5:32 AM Mark Thomas via jsp-dev <jsp-dev@xxxxxxxxxxx> wrote:

Apologies for the flip-flopping but on further reflection I think there
are good reasons to keep this deprecation.

The old constructor was only used by the API and is no longer used (the
API now uses the new constructor).

It seems unlikely that a container or application would be using the old
constructor but if they are, they need to switch to using the new one
and the deprecation is a way of getting that message across.

While null is valid for the query string, the query string must be
provided if available. It boils down to:

With old constructor:

String qs = (String)
getRequest().getAttribute("jakarta.servlet.error.query_string");
if (qs == null) {
     return new ErrorData((Throwable)
getRequest().getAttribute("jakarta.servlet.error.exception"),
             ((Integer)
getRequest().getAttribute("jakarta.servlet.error.status_code")).intValue(),
             (String)
getRequest().getAttribute("jakarta.servlet.error.request_uri"),
             (String)
getRequest().getAttribute("jakarta.servlet.error.servlet_name"));
} else {
     return new ErrorData((Throwable)
getRequest().getAttribute("jakarta.servlet.error.exception"),
             ((Integer)
getRequest().getAttribute("jakarta.servlet.error.status_code")).intValue(),
             (String)
getRequest().getAttribute("jakarta.servlet.error.request_uri"),
             (String)
getRequest().getAttribute("jakarta.servlet.error.servlet_name"),
             (String)
getRequest().getAttribute("jakarta.servlet.error.query_string"));
}


With just the new constructor:

return new ErrorData((Throwable)
getRequest().getAttribute("jakarta.servlet.error.exception"),
         ((Integer)
getRequest().getAttribute("jakarta.servlet.error.status_code")).intValue(),
         (String)
getRequest().getAttribute("jakarta.servlet.error.request_uri"),
         (String)
getRequest().getAttribute("jakarta.servlet.error.servlet_name"),
         (String)
getRequest().getAttribute("jakarta.servlet.error.query_string"));


I can't think of any scenario where it helps to keep the old
constructor. I'll give it another week or so for folks to comment but my
intention at this point is to revert the removal of the deprecation.

Mark



On 14/10/2023 23:37, Mark Thomas via jsp-dev wrote:
> On 09/10/2023 11:07, Paul Nicolucci via jsp-dev wrote:
>
>> Why don't we keep both constructors if null is perfectly valid for the
>> queryString?
>
> Good question. I was probably thinking in terms of replacing the old
> constructor with the new one.
>
> Since queryString looks to be the only optional value passed to
> ErrorData, I can see the benefit in keeping the existing constructor.
>
> If no other opinions are expressed, we can remove the deprecation in a
> week or so.
>
> Mark
> _______________________________________________
> jsp-dev mailing list
> jsp-dev@xxxxxxxxxxx
> To unsubscribe from this list, visit
> https://www.eclipse.org/mailman/listinfo/jsp-dev
_______________________________________________
jsp-dev mailing list
jsp-dev@xxxxxxxxxxx
To unsubscribe from this list, visit https://www.eclipse.org/mailman/listinfo/jsp-dev