Add HTTP timeout to geocode provider options

[chrismayer]

Adds a configurable HTTP timeout (in ms) to the geocode providerOptions, which defaults to 15000 milliseconds.

[sronveaux]

Hi @chrismayer,

Thanks for tackling this problem which seemed to be a test problem but is, as this PR confirms, way broader than that!

However, I personally think the timeout parameter should be placed somewhere else as you will see in the comments I've written in the review.
From my point of view, providerOptions should be used only to gather the parameters specific to the geocoding service you're going to use (the provider), be it osm, photon or opencage for now.
It complicates things a little bit but I think having a separate timeout parameter would be more logical. I don't know what @fschmenger thinks about it or whether you agree on this...

If you wanted to try what I added inside comments, you'd also need to add the following lines:

In GeocoderController.js: change the constructor to this:

  constructor (providerName, options, timeout) {
    this.options = options;
    this.timeout = timeout;
    this.abortController = null;

And replace Geocoder.vue <script> with the following:

<script>
import { useMap } from '@/composables/Map';
import { useColorTheme } from '@/composables/ColorTheme';
import { GeocoderController } from './GeocoderController';
import { applyTransform } from 'ol/extent';
import { getTransform, fromLonLat } from 'ol/proj';
import ViewAnimationUtil from '@/util/ViewAnimation';
import axios from 'axios';

export default {
  name: 'wgu-geocoder-input',
  props: {
    icon: { type: String, required: false, default: 'md:search' },
    width: { type: Number, required: false, default: undefined },
    minWidth: { type: Number, required: false, default: 175 },
    maxWidth: { type: Number, required: false, default: 300 },
    rounded: { type: Boolean, required: false, default: true },
    autofocus: { type: Boolean, required: false, default: true },
    clearable: { type: Boolean, required: false, default: true },
    persistentHint: { type: Boolean, required: false, default: true },
    debug: { type: Boolean, required: false, default: false },
    minChars: { type: Number, required: false, default: 3 },
    queryDelay: { type: Number, required: false, default: 300 },
    timeout: { type: Number, required: false, default: 15000 },
    provider: { type: String, required: false, default: 'osm' },
    providerOptions: { type: Object, required: false, default: function () { return {}; } }
  },
  setup () {
    const { map } = useMap();
    const { isDarkTheme, isPrimaryDark, isPrimaryDarkWithLightTheme } = useColorTheme();

    return { map, isDarkTheme, isPrimaryDark, isPrimaryDarkWithLightTheme };
  },
  data () {
    return {
      results: null,
      lastQueryStr: '',
      noFilter: true,
      selecting: false,
      selected: null,
      hideSearch: true,
      debounceTimeout: null
    }
  },
  computed: {
    resultItems () {
      const items = [];
      if (!this.results) {
        return items;
      }
      this.trace(`computed.resultItems() - cur results len=${this.results.length}`);

      // Convert results to v-combobox (title, value) Items
      this.results.forEach(result => {
        this.trace(`add to this.items: ${result.address.name}`);
        items.push({ title: result.address.name, value: result });
      });

      return items;
    }
  },
  mounted () {
    // Setup GeocoderController to which we delegate Provider and query-handling
    this.geocoderController = new GeocoderController(this.provider, this.providerOptions, this.timeout);
  },
  unmounted () {
    if (this.debounceTimeout) {
      clearTimeout(this.debounceTimeout);
      this.debounceTimeout = null;
    }

    this.geocoderController.destroy();
  },
  methods: {
    trace (str) {
      this.debug && console && console.info(str);
    },
    toggle () {
      // Show/hide search combobox
      this.hideSearch = !this.hideSearch;
    },
    // Query by string - should return list of selection items (adresses) for ComboBox
    querySelections (queryStr) {
      if (this.debounceTimeout) {
        clearTimeout(this.debounceTimeout);
      }
      this.debounceTimeout = setTimeout(() => {
        // Let Geocoder Provider do the query
        // items (item.title fields) will be shown in combobox dropdown suggestions
        this.trace(`geocoderController.query: ${queryStr}`);
        this.geocoderController.query(queryStr)
          .then(results => this.onQueryResults(results))
          .catch(err => {
            if (!axios.isCancel(err)) {
              this.onQueryError(err);
            }
          });
        this.debounceTimeout = null;
      }, this.queryDelay);
    },
    onQueryResults (results) {
      // Handle query results from GeocoderController
      this.results = null;

      if (!results || results.length === 0) {
        return;
      }

      // ASSERT: results is defined and at least one result
      this.trace(`results ok: len=${results.length}`);
      this.results = results;
    },
    onQueryError (err) {
      if (err) {
        this.trace(`onQueryResult error: ${err}`);
      }
    },
    // Input string value changed
    search (queryStr) {
      if (this.selecting) {
        // Selection in progress
        this.trace('selection in progress...');
        return;
      }
      if (!queryStr || queryStr.length === 0) {
        // Query reset
        this.trace('queryStr none');
        this.results = null;
        return;
      }
      // ASSERTION: queryStr is valid
      queryStr = queryStr.trim();

      // Only query if minimal number chars typed and querystring has changed
      if (queryStr.length >= this.minChars && queryStr !== this.lastQueryStr) {
        this.querySelections(queryStr);
        this.lastQueryStr = queryStr;
      }
    }
  },
  watch: {
    // User has selected entry from suggested items
    selected (item) {
      if (!item || !Object.prototype.hasOwnProperty.call(item, 'title') || !Object.prototype.hasOwnProperty.call(item, 'value')) {
        return;
      }
      this.selecting = true;
      this.trace(`selected=${item.title}`);

      // Position Map on result
      const result = item.value;
      const mapProjection = this.map.getView().getProjection();
      const viewAnimationUtil = new ViewAnimationUtil(this.$appConfig.viewAnimation);

      // Prefer zooming to bounding box if present in result
      if (Object.prototype.hasOwnProperty.call(result, 'boundingbox')) {
        // Result with bounding box.
        // bbox is in EPSG:4326, needs to be transformed to Map Projection (e.g. EPSG:3758)
        const extent = applyTransform(result.boundingbox, getTransform('EPSG:4326', mapProjection));
        viewAnimationUtil.to(this.map.getView(), extent);
      } else {
        // No bbox in result: center on lon/lat from result and zoom in
        const coords = fromLonLat([result.lon, result.lat], mapProjection);
        viewAnimationUtil.to(this.map.getView(), coords);
      }
      this.selecting = false;
    }
  }
};
</script>

Some changes are due to timeout being already defined inside data. However, this variable is in all cases better named debounceTimeout.

Please also note this solution still has a limitation due to the way Axios manages timeouts. The timeout parameter monitors the time between the request is sent and the first byte of the response is received. If the server sends headers but then waits a very long time before sending the reply, the timeout will not occur...
I don't know if this could occur with those geocoding providers and the proposed solution is totally fine for now IMHO, but this is important to say here in case some problems occur due to slow responses from a server...

Thanks again for taking this one, this will be a very nice addition which will harden Wegue a bit more!

[sronveaux]

Suggested change

	| queryDelay | Delay in MS before a query is triggered | `"queryDelay": 200` |
	| debug | Boolean value to enable debug logs | `"debug": false` |
	| queryDelay | Delay in MS before a query is triggered | `"queryDelay": 200` |
	| timeout | Timeout in MS before a query request is aborted. Defaults to `15000` | `"timeout": 15000` |
	| debug | Boolean value to enable debug logs | `"debug": false` |

[sronveaux]

Suggested change

	| providerOptions | Optional options which are passed to the geocoder provider | `"providerOptions": {"lang": "en-US", "countrycodes": "", "limit": 6, timeout: 8000}` |
	| providerOptions | Optional options which are passed to the geocoder provider | `"providerOptions": {"lang": "en-US", "countrycodes": "", "limit": 6}` |

[sronveaux]

Suggested change

	timeout: this.options.timeout || 15000
	timeout: this.timeout

[fschmenger]

I didn't have time to dig into this deeply. From a design point my impression is that separating timeout from other service dependent parameters is preferred- e.g. we already declaring options like queryDelay separately.

[chrismayer]

Thanks for your reviews @sronveaux and @fschmenger. I followed your suggestion and separated the HTTP timeout from other service dependent parameters. Therefore I renamed timeout to debounceTimeout and introduced a new prop httpTimeout, which is passed into the GeocoderController constructor.