Skip to content

Enhance response detail fetching to include geometries and improve ca… #1204

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.

Sign up for GitHub

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

Merged
gibahjoe merged 4 commits into from
Jun 10, 2026
Merged

Enhance response detail fetching to include geometries and improve ca… #1204

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d134d38
Enhance response detail fetching to include geometries and improve ca…
gibahjoe Jun 8, 2026
5701273
Refactor response detail fetching to remove geometry inclusion and im…
gibahjoe Jun 8, 2026
ebd0eb1
Refactor response detail fetching to simplify geometry handling and i…
gibahjoe Jun 8, 2026
beae29b
Merge branch 'main' into 2658-stop-over-fetching-response-details
gibahjoe Jun 9, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 57 additions & 53 deletions src/controllers/resultsController.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,65 +173,69 @@ export const fieldToColumnMapping = ({ columns }) => {
* @param {Function} next - Next middleware function
* @returns {void}
*/
export function setupTableParams (req, res, next) {
if (req.locals.template !== failedFileRequestTemplate && req.locals.template !== failedUrlRequestTemplate) {
const responseDetails = req.locals.responseDetails
// Optionally filter out all non - error rows from dataset
let rows = responseDetails.getRowsWithVerboseColumns(false)
// remove any issues that aren't of severity error
rows = rows.map((row) => {
const { columns, ...rest } = row

const columnsOnlyErrors = Object.fromEntries(Object.entries(columns).map(([key, value]) => {
let error
if (value.error && value.error.severity === 'error' && value.error.responsibility !== 'internal') {
error = value.error
}
const newValue = {
...value,
error
export async function setupTableParams (req, res, next) {
try {
if (req.locals.template !== failedFileRequestTemplate && req.locals.template !== failedUrlRequestTemplate) {
const responseDetails = req.locals.responseDetails
// Optionally filter out all non - error rows from dataset
let rows = responseDetails.getRowsWithVerboseColumns(false)
// remove any issues that aren't of severity error
rows = rows.map((row) => {
const { columns, ...rest } = row

const columnsOnlyErrors = Object.fromEntries(Object.entries(columns).map(([key, value]) => {
let error
if (value.error && value.error.severity === 'error' && value.error.responsibility !== 'internal') {
error = value.error
}
const newValue = {
...value,
error
}
return [key, newValue]
}))

return {
...rest,
columns: columnsOnlyErrors
}
return [key, newValue]
}))
})

return {
...rest,
columns: columnsOnlyErrors
const fieldToColumn = rows.length > 0 ? fieldToColumnMapping(rows[0]) : new Map()
const columnToField = new Map()
for (const [k, v] of fieldToColumn.entries()) {
columnToField.set(v, k)
}
})

const fieldToColumn = rows.length > 0 ? fieldToColumnMapping(rows[0]) : new Map()
const columnToField = new Map()
for (const [k, v] of fieldToColumn.entries()) {
columnToField.set(v, k)
}

const { leading: leadingFields, trailing: trailingFields } = splitByLeading({ fields: responseDetails.getFields() })
// NOTE: the column field log alters the field names (converts '_' -> '-', most of the time 🤷‍♂️), but we want
// the original CSV column names because that's what users expect
const orderedFields = [...leadingFields, ...trailingFields]
const columns = orderedFields
const fields = orderedFields
req.locals.tableParams = {
columns,
fields,
rows,
columnNameProcessing: 'none',
mapping: columnToField
const { leading: leadingFields, trailing: trailingFields } = splitByLeading({ fields: responseDetails.getFields() })
// NOTE: the column field log alters the field names (converts '_' -> '-', most of the time 🤷‍♂️), but we want
// the original CSV column names because that's what users expect
const orderedFields = [...leadingFields, ...trailingFields]
const columns = orderedFields
const fields = orderedFields
req.locals.tableParams = {
columns,
fields,
rows,
columnNameProcessing: 'none',
mapping: columnToField
}
req.locals.geometries =
req.locals.datasetTypology === 'geography'
? await responseDetails.getGeometries()
: null
// pagination is on the 'table' tab, so we want to ensure clicking those
// links takes us to a page with the table tab *selected*
const { pageNumber } = req.parsedParams
const pagination = responseDetails.getPagination(pageNumber, { hash: '#table-tab' })
req.locals.pagination = pagination
req.locals.id = req.params.id
req.locals.lastPage = `/check/status/${req.params.id}`
}
req.locals.geometries =
req.locals.datasetTypology === 'geography'
? responseDetails.getGeometries()
: null
// pagination is on the 'table' tab, so we want to ensure clicking those
// links takes us to a page with the table tab *selected*
const { pageNumber } = req.parsedParams
const pagination = responseDetails.getPagination(pageNumber, { hash: '#table-tab' })
req.locals.pagination = pagination
req.locals.id = req.params.id
req.locals.lastPage = `/check/status/${req.params.id}`
next()
} catch (error) {
next(error)
}
next()
}

export function setupError (req, res, next) {
Expand Down
119 changes: 3 additions & 116 deletions src/models/requestData.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import * as v from 'valibot'
import logger from '../utils/logger.js'
import { types } from '../utils/logging.js'
import axios from 'axios'
import config from '../../config/index.js'
import ResponseDetails from './responseDetails.js'
Expand Down Expand Up @@ -37,7 +36,6 @@ export default class ResultData {
*/
async fetchResponseDetails (pageOffset = 0, limit = 50, opts = { severity: undefined }) {
v.parse(ResponseDetailsOptions, opts)

const url = new URL(`${config.asyncRequestApi.url}/${config.asyncRequestApi.requestsEndpoint}/${this.id}/response-details`)
url.searchParams.append('offset', pageOffset * limit)
url.searchParams.append('limit', limit)
Expand All @@ -51,30 +49,16 @@ export default class ResultData {
url.searchParams.append('jsonpath', `$.issue_logs[*].severity=="${opts.severity}"`)
}

// we do initial request, check how many records there are via 'x-pagination-total-results' header
// and if fetch the rest if needed
const response = await axios.get(url, { timeout: 30000 })
const totalResults = Number.parseInt(response.headers['x-pagination-total-results'])
const responses = [...response.data]
if (Number.isInteger(totalResults) && totalResults > response.data.length) {
const urlTemplate = new URL(url)
urlTemplate.searchParams.delete('offset')
urlTemplate.searchParams.delete('limit')

const paginationOpts = { limit, offset: response.data.length, maxOffset: Number.isInteger(totalResults) ? totalResults : 100 }
const restResponses = await fetchPaginated(url, paginationOpts)
responses.push(...restResponses.flatMap(resp => resp.data))
}

// we're not using x-pagination-offset and x-pagination-limit headers, because we fetched
// all the records already, so there's no need for pagination controls on the table
const pagination = {
totalResults: `${totalResults}`,
offset: '0',
limit: `${totalResults}`
offset: `${pageOffset * limit}`,
limit: `${limit}`
}

return new ResponseDetails(this.id, responses, pagination, this.getColumnFieldLog())
return new ResponseDetails(this.id, response.data, pagination, this.getColumnFieldLog())
}

isFailed () {
Expand Down Expand Up @@ -196,100 +180,3 @@ export default class ResultData {
return this.response.data.plugin ?? null
}
}

/**
* Returns a generator of offset values.
*
* @param {number} limit
* @param {number} offset
* @param {number} maxOffset
*/
function * offsets (limit, offset, maxOffset) {
let currentOffset = offset
while (currentOffset < maxOffset) {
yield currentOffset
currentOffset += limit
}
}

/**
*
* @param {number} numTasks max number of tasks to run
* @param {Object} gen offset generator
* @param {Function} taskFactory (taskIndex, offset) => Promise<>
* @returns {Promise[]}
*/
function startRequests (numTasks, gen, taskFactory) {
const tasks = []
for (let i = 0; i < numTasks; ++i) {
const offsetItem = gen.next()
if (!offsetItem.done) {
const p = taskFactory(i, offsetItem.value)
tasks.push(p)
} else {
break
}
}
return tasks
}

/**
* Given a task factor function, executes a number of async tasks in parallel,
* but only at most `options.concurrency` tasks are in flight.
*
* Note: the tasks should be IO bound.
*
* If any of the tasks fail, the whole operation fails (in other words:
* no partial results).
*
* @param {Object} options
* @returns {Promise<Object[][]>}
*/
async function fetchBatched (options) {
// Note: trying more involved strategy of launching requests by using Promise.any()
// and trying to immedieately replace that one completed promise with a new one
// didn't really behave as expected - work was happening mostly in a single promise.
// This one's simpler and seems to actually do what expected.
const { concurrency, taskFn, offsetInfo } = options
const results = []
const gen = offsets(offsetInfo.limit, offsetInfo.offset, offsetInfo.maxOffset)
const newTask = async (index, offset) => {
logger.debug('fetchBatched(): starting task', { task: index, offset, type: types.DataFetch })
const p = taskFn(offset).then((val) => {
logger.debug('fetchBatched(): finishing task', { task: index, offset, type: types.DataFetch })
return { val, index, offset }
})
return p
}

let promises = startRequests(concurrency, gen, newTask)

do {
const completed = await Promise.all(promises)
results.push(...completed)
promises = startRequests(concurrency, gen, newTask)
logger.debug(`fetchBatched(): completed ${completed.length} tasks`, { type: types.DataFetch })
} while (promises.length > 0)

logger.info(`fetchBatched(): completed ${results.length} requests`, { type: types.DataFetch })
results.sort((r1, r2) => r1.offset - r2.offset)
return results.map(r => r.val)
}

/**
*
* @param {URL} url url
* @param {Object} options
* @returns {Promise<Object[]>}
*/
export const fetchPaginated = async (url, { limit, offset, maxOffset }) => {
const taskFn = async (offset) => {
const thisUrl = new URL(url)
thisUrl.searchParams.set('offset', offset)
thisUrl.searchParams.set('limit', limit)
const result = await axios.get(thisUrl, { timeout: 10000 })
return result
}

return await fetchBatched({ concurrency: 4, taskFn, offsetInfo: { limit, offset, maxOffset } })
}
92 changes: 46 additions & 46 deletions src/models/responseDetails.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import { getVerboseColumns } from '../utils/getVerboseColumns.js'
import logger from '../utils/logger.js'
import { types } from '../utils/logging.js'
import { pagination } from '../utils/pagination.js'
import axios from 'axios'
import config from '../../config/index.js'

/**
* @typedef {Object} PaginationOptions
Expand Down Expand Up @@ -32,6 +34,8 @@ import { pagination } from '../utils/pagination.js'
*/
export default class ResponseDetails {
#cachedFields
#cachedGeometries
#hasFetchedGeometries = false

constructor (id, response, pagination, columnFieldLog) {
this.id = id
Expand Down Expand Up @@ -161,35 +165,57 @@ export default class ResponseDetails {
*
* @returns {any[] | undefined }
*/
getGeometries () {
const rows = this.getRows()
if (rows.length === 0) {
async getGeometries () {
if (this.#hasFetchedGeometries) {
return this.#cachedGeometries
}

this.#cachedGeometries = await this.#fetchGeometries()
this.#hasFetchedGeometries = true
return this.#cachedGeometries
}

async #fetchGeometries () {
if (!this.id) {
return undefined
}

const item = rows[0]
const getGeometryValue = this.#makeGeometryGetter(item)
if (!getGeometryValue) {
logger.debug('could not create geometry getter', {
type: types.App,
requestId: this.id
const url = new URL(`${config.asyncRequestApi.url}/${config.asyncRequestApi.requestsEndpoint}/${this.id}/geometries`)
let response
try {
response = await axios.get(url, { timeout: 30000 })
} catch (error) {
logger.warn('failed to fetch response geometries', {
type: types.DataFetch,
requestId: this.id,
errorMessage: error.message
})
return undefined
}
const totalResults = Number.parseInt(response.headers?.['x-pagination-total-results'])
const geometries = response.data

if (!Array.isArray(geometries)) {
return undefined
}

const limit = Number.parseInt(response.headers?.['x-pagination-limit']) || geometries.length || 500

if (!Number.isInteger(totalResults) || geometries.length >= totalResults) {
return geometries.length > 0 ? geometries : undefined
}

const geometries = []
for (const item of rows) {
const geometry = getGeometryValue(item)
if (geometry && geometry.trim() !== '') {
geometries.push(geometry)
for (let offset = geometries.length; offset < totalResults; offset += limit) {
const pageUrl = new URL(url)
pageUrl.searchParams.set('offset', offset)
pageUrl.searchParams.set('limit', limit)
const page = await axios.get(pageUrl, { timeout: 30000 })
if (!Array.isArray(page.data)) {
Comment on lines +208 to +213

@coderabbitai coderabbitai Bot Jun 9, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Paginated geometry fetch can fail the entire results page on later-page request errors.

Line 212 performs follow-up axios.get calls outside a try/catch. If page 2+ fetch fails, getGeometries() rejects and setupTableParams aborts, so users can lose the whole results page even when table data already loaded.

💡 Suggested fix 
     for (let offset = geometries.length; offset < totalResults; offset += limit) {
       const pageUrl = new URL(url)
       pageUrl.searchParams.set('offset', offset)
       pageUrl.searchParams.set('limit', limit)
-      const page = await axios.get(pageUrl, { timeout: 30000 })
+      let page
+      try {
+        page = await axios.get(pageUrl, { timeout: 30000 })
+      } catch (error) {
+        logger.warn('failed to fetch paginated response geometries', {
+          type: types.DataFetch,
+          requestId: this.id,
+          errorMessage: error.message
+        })
+        break
+      }
       if (!Array.isArray(page.data)) {
         break
       }
       geometries.push(...page.data)
     }
 
-    return geometries
+    return geometries.length > 0 ? geometries : undefined
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
for (let offset = geometries.length; offset < totalResults; offset += limit) {
const pageUrl = new URL(url)
pageUrl.searchParams.set('offset', offset)
pageUrl.searchParams.set('limit', limit)
const page = await axios.get(pageUrl, { timeout: 30000 })
if (!Array.isArray(page.data)) {
for (let offset = geometries.length; offset < totalResults; offset += limit) {
const pageUrl = new URL(url)
pageUrl.searchParams.set('offset', offset)
pageUrl.searchParams.set('limit', limit)
let page
try {
page = await axios.get(pageUrl, { timeout: 30000 })
} catch (error) {
logger.warn('failed to fetch paginated response geometries', {
type: types.DataFetch,
requestId: this.id,
errorMessage: error.message
})
break
}
if (!Array.isArray(page.data)) {
break
}
geometries.push(...page.data)
}
return geometries.length > 0 ? geometries : undefined
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/models/responseDetails.js` around lines 208 - 213, getGeometries()
performs additional axios.get calls in the pagination loop without error
handling, so a failure on page 2+ will reject the whole call and abort
setupTableParams; wrap the per-page fetch (the axios.get inside the for loop) in
a try/catch, handle errors by logging the error and either skipping that page
(continue) or breaking gracefully while still returning the geometries collected
so far, and ensure the function resolves with partial results instead of
rethrowing; update any callers (e.g., setupTableParams) expectations if needed
to accept partial geometry arrays.

break
}
geometries.push(...page.data)
}
logger.debug('getGetometries()', {
type: types.App,
requestId: this.id,
geometryCount: geometries.length,
rowCount: rows.length
})

return geometries
}

Expand Down Expand Up @@ -236,30 +262,4 @@ export default class ResponseDetails {
items
}
}

/**
* Detects where geometry is stored in the item and returns a function to extract geometry value.
* It's caller's responsibility to handle situations where the getter couldn't be returned.
* For most common use cases, we can omit displaying the map.
*
* @param {Object} item - Data item containing geometry information
* @returns {Function|undefined} Function that takes an item and returns a geometry string, or undefined if no geometry found
*/
#makeGeometryGetter (item) {
/*
The api seems to sometimes respond with weird casing, it can be camal case, all lower or all upper
I'll implement a fix here, but hopefully infa will be addressing it on the backend to
*/
const trow = item.transformed_row
if (trow) {
const key = trow.find(obj => obj.field === 'geometry' || obj.field === 'point')?.field
const getter = (row) => {
const geometry = row.transformed_row?.find(obj => obj.field === key)
return geometry?.value
}
return getter
}

return undefined
}
}
Loading
Loading