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

Queries that find no results not returning an error message #6907

Closed
sdlime opened this issue Jun 9, 2023 · 7 comments
Closed

Queries that find no results not returning an error message #6907

sdlime opened this issue Jun 9, 2023 · 7 comments
Milestone

Comments

@sdlime
Copy link
Member

sdlime commented Jun 9, 2023

Expected behavior and actual behavior.

In 8.0.1 I'm seeing queries return no results but not producing the expected error message. Depending on how the query is configured to return content that can be a blank response instead of the usual error message such as "msQueryByPoint(): Search returned no results. No matching record(s) found.". There is no crash or entry in the web server error log. When using a template via an OUTPUTFORMAT, that processing completes - although it shouldn't - the error should be trapped beforehand.

I've seen the problem with mode query and nquery.

Steps to reproduce the problem.

Take any mode=query or mode=nquery URL that should find no matches. I just curious if anyone can replicate...

Operating system

SuSE 15.5

MapServer version and installation method

8.0.1 installed from source (actually a recent pull from branch-8-0 so it's a little newer than 8.0.1).

@sdlime sdlime changed the title Queries that find no results not returning error messages Queries that find no results not returning and error message Jun 9, 2023
@geographika
Copy link
Member

@sdlime - I think this was me with the changes in #6268. Adding a DEBUG 5 level to the layer or map should get the errors in the logs. The main reasoning was that logs were filling up (at a rapid rate) with any request that returned no results, and it couldn't be turned off as they were errors.

I was also running into issues when selecting records as GeoJSON - any WFS query that returned 0 results would then return an XML error response. I'd not fully considered the implications on the CGI API though. 0 results seemed to be a valid response to a query, and errors reserved for invalid queries.

@sdlime
Copy link
Member Author

sdlime commented Jun 12, 2023

Ok, I get it. The only reason I noticed is that I had a couple of web maps that were relying on that error to catch the 0 result case. I can certainly work around it and test for valid responses.

That said, we don't have a good way to deal with 0 results in the templated output. There is an EMPTY keyword in the WEB object that provides a link to redirect to when no results are found. I just tried that with 8.0.1 and it doesn't work. For the templated OUTPUTFORMAT we'd almost need to add a new tag, something like [emptyresultset] or [resultset empty="true] to hold content for that use case (I kinda like the latter). That, plus reviving EMPTY would have us covered I think.

@jmckenna jmckenna added this to the 8.2.0 Release milestone Jun 12, 2023
@sdlime
Copy link
Member Author

sdlime commented Sep 7, 2023

This won't be as easy as I hoped... The redirection to web->empty happens in msCGIWriteError() and looks for the MS_NOTFOUND error code. That code is still used in places, but in the query functions (mapquery.c) only in msQueryByIndex(). Makes me wonder if it should be removed there too... There are still a smattering of references to MS_NOTFOUND in mapwfs.cpp and mapwms.cpp. @geographika, what do you think?

@geographika
Copy link
Member

The MS_NOTFOUND in mapquery.c seems to hide/replace several other types of error e.g. errors in msLayerGetShape are replaced with MS_NOTFOUND

  status = msLayerGetShape(lp, &shape, &record);
  if(status != MS_SUCCESS) {
    msSetError(MS_NOTFOUND, "Not valid record request.", "msQueryByIndex()");
    return(MS_FAILURE);
  }

Could the original error be left in this case?
In the other cases maybe a more specific error could be returned?

  shape.classindex = msShapeGetClass(lp, map, &shape, NULL, 0);
  if(!(lp->template) && ((shape.classindex == -1) || (lp->class[shape.classindex]->status == MS_OFF))) { /* not a valid shape */
    msSetError(MS_NOTFOUND, "Requested shape not valid against layer classification scheme.", "msQueryByIndex()");
    msFreeShape(&shape);

These cases would then be handled in the ERROR template.
It still leaves the actual cases where there are no records returned:

Before #6268:

  msSetError(MS_NOTFOUND, "No matching record(s) found.", "msQueryByShape()");
  return(MS_FAILURE);

After:

  if (map->debug >= MS_DEBUGLEVEL_V) {
      msDebug("msQueryByShape(): No matching record(s) found.");
  }
  return(MS_SUCCESS);

Is there anyway to take advantage of the [nr] template option listed in https://mapserver.org/mapfile/template.html to handle these cases?

[nr]

    Total number of results. Useful in web header and footers. Available only when processing query results.

@sdlime
Copy link
Member Author

sdlime commented Sep 7, 2023

Good catch on the [nr] tag, I had forgotten about that. Computing the total across all layers is easy but if there's already a function out there to do that then that would be helpful. I think I just need to insert a bit of code near line 1680 in mapservutil.c to catch the no results case when web->empty is set and then throw the old error message.

I think you're right that some of the error conditions in msQueryByIndex() shouldn't be using MS_NOTFOUND. I'll try to address that in my pull request.

Thanks for the quick feedback!

@jmckenna jmckenna changed the title Queries that find no results not returning and error message Queries that find no results not returning an error message Dec 11, 2023
@sdlime
Copy link
Member Author

sdlime commented Jan 29, 2024

Quick update... I have a patch working, just trying to press a new development box into service to finish it up... --Steve

@jmckenna
Copy link
Member

great, thanks Steve

sdlime added a commit to sdlime/mapserver that referenced this issue Feb 18, 2024
…global means of configuring via config file (MS_EMPTY).
sdlime added a commit to sdlime/mapserver that referenced this issue Feb 20, 2024
sdlime added a commit to sdlime/mapserver that referenced this issue Feb 20, 2024
sdlime added a commit that referenced this issue Mar 14, 2024
Fixed regression noted in #6907 and added global config option for web empty. Added global error config option referenced in #6968.
@sdlime sdlime closed this as completed Mar 14, 2024
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

No branches or pull requests

3 participants