Skip to content

Commit

Permalink
[Maps] fix vector tile load errors not displayed in legend (elastic#1…
Browse files Browse the repository at this point in the history
…30395)

* [Maps] fix vector tile load errors not displayed in legend

* revert unneeded change

* update API docs

* add error integration test

* [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix'

* eslint and fix jest test

* [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix'

* cleanup

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
2 people authored and kertal committed May 24, 2022
1 parent bf84f43 commit cd71eac
Show file tree
Hide file tree
Showing 17 changed files with 258 additions and 34 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ kibanaResponseFactory: {
message: string | Error;
attributes?: ResponseErrorAttributes | undefined;
}>;
customError: (options: CustomHttpResponseOptions<ResponseError>) => KibanaResponse<string | Error | {
customError: (options: CustomHttpResponseOptions<ResponseError | Buffer | Stream>) => KibanaResponse<string | Error | Buffer | Stream | {
message: string | Error;
attributes?: ResponseErrorAttributes | undefined;
}>;
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/http/router/response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ const errorResponseFactory = {
* Creates an error response with defined status code and payload.
* @param options - {@link CustomHttpResponseOptions} configures HTTP response headers, error message and other error details to pass to the client
*/
customError: (options: CustomHttpResponseOptions<ResponseError>) => {
customError: (options: CustomHttpResponseOptions<ResponseError | Buffer | Stream>) => {
if (!options || !options.statusCode) {
throw new Error(
`options.statusCode is expected to be set. given options: ${options && options.statusCode}`
Expand Down
2 changes: 1 addition & 1 deletion src/core/server/server.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1464,7 +1464,7 @@ export const kibanaResponseFactory: {
message: string | Error;
attributes?: ResponseErrorAttributes | undefined;
}>;
customError: (options: CustomHttpResponseOptions<ResponseError>) => KibanaResponse<string | Error | {
customError: (options: CustomHttpResponseOptions<ResponseError | Buffer | Stream>) => KibanaResponse<string | Error | Buffer | Stream | {
message: string | Error;
attributes?: ResponseErrorAttributes | undefined;
}>;
Expand Down
2 changes: 1 addition & 1 deletion x-pack/plugins/maps/public/actions/data_request_actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ export function syncDataForLayerId(layerId: string | null, isForceRefresh: boole
};
}

function setLayerDataLoadErrorStatus(layerId: string, errorMessage: string | null) {
export function setLayerDataLoadErrorStatus(layerId: string, errorMessage: string | null) {
return {
type: SET_LAYER_ERROR_STATUS,
isInErrorState: errorMessage !== null,
Expand Down
1 change: 1 addition & 0 deletions x-pack/plugins/maps/public/actions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export {
cancelAllInFlightRequests,
fitToLayerExtent,
fitToDataBounds,
setLayerDataLoadErrorStatus,
} from './data_request_actions';
export {
closeOnClickTooltip,
Expand Down
11 changes: 10 additions & 1 deletion x-pack/plugins/maps/public/classes/util/geo_tile_utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@
* 2.0.
*/

import { parseTileKey, getTileBoundingBox, expandToTileBoundaries } from './geo_tile_utils';
import {
getTileKey,
parseTileKey,
getTileBoundingBox,
expandToTileBoundaries,
} from './geo_tile_utils';

it('Should parse tile key', () => {
expect(parseTileKey('15/23423/1867')).toEqual({
Expand All @@ -16,6 +21,10 @@ it('Should parse tile key', () => {
});
});

it('Should get tile key', () => {
expect(getTileKey(45, 120, 10)).toEqual('10/853/368');
});

it('Should convert tile key to geojson Polygon', () => {
const geometry = getTileBoundingBox('15/23423/1867');
expect(geometry).toEqual({
Expand Down
8 changes: 8 additions & 0 deletions x-pack/plugins/maps/public/classes/util/geo_tile_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,14 @@ export function parseTileKey(tileKey: string): {
return { x, y, zoom, tileCount };
}

export function getTileKey(lat: number, lon: number, zoom: number): string {
const tileCount = getTileCount(zoom);

const x = longitudeToTile(lon, tileCount);
const y = latitudeToTile(lat, tileCount);
return `${zoom}/${x}/${y}`;
}

function sinh(x: number): number {
return (Math.exp(x) - Math.exp(-x)) / 2;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
mapExtentChanged,
mapReady,
setAreTilesLoaded,
setLayerDataLoadErrorStatus,
setMapInitError,
setMouseCoordinates,
updateMetaFromTiles,
Expand Down Expand Up @@ -86,6 +87,12 @@ function mapDispatchToProps(dispatch: ThunkDispatch<MapStoreState, void, AnyActi
updateMetaFromTiles(layerId: string, features: TileMetaFeature[]) {
dispatch(updateMetaFromTiles(layerId, features));
},
clearTileLoadError(layerId: string) {
dispatch(setLayerDataLoadErrorStatus(layerId, null));
},
setTileLoadError(layerId: string, errorMessage: string) {
dispatch(setLayerDataLoadErrorStatus(layerId, errorMessage));
},
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ export interface Props {
updateMetaFromTiles: (layerId: string, features: TileMetaFeature[]) => void;
featureModeActive: boolean;
filterModeActive: boolean;
setTileLoadError(layerId: string, errorMessage: string): void;
clearTileLoadError(layerId: string): void;
}

interface State {
Expand Down Expand Up @@ -205,8 +207,15 @@ export class MbMap extends Component<Props, State> {
this._tileStatusTracker = new TileStatusTracker({
mbMap,
getCurrentLayerList: () => this.props.layerList,
updateTileStatus: (layer: ILayer, areTilesLoaded: boolean) => {
updateTileStatus: (layer: ILayer, areTilesLoaded: boolean, errorMessage?: string) => {
this.props.setAreTilesLoaded(layer.getId(), areTilesLoaded);

if (errorMessage) {
this.props.setTileLoadError(layer.getId(), errorMessage);
} else {
this.props.clearTileLoadError(layer.getId());
}

this._queryForMeta(layer);
},
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,11 @@ function createMockMbDataEvent(mbSourceId: string, tileKey: string): unknown {
dataType: 'source',
tile: {
tileID: {
canonical: {
x: 80,
y: 10,
z: 5,
},
key: tileKey,
},
},
Expand Down Expand Up @@ -133,7 +138,7 @@ describe('TileStatusTracker', () => {
},
});

expect(mockMbMap.listeners.length).toBe(3);
expect(mockMbMap.listeners.length).toBe(4);
tileStatusTracker.destroy();
expect(mockMbMap.listeners.length).toBe(0);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,21 @@

import type { Map as MapboxMap, MapSourceDataEvent } from '@kbn/mapbox-gl';
import _ from 'lodash';
import { i18n } from '@kbn/i18n';
import { ILayer } from '../../classes/layers/layer';
import { SPATIAL_FILTERS_LAYER_ID } from '../../../common/constants';
import { getTileKey } from '../../classes/util/geo_tile_utils';

interface MbTile {
// references internal object from mapbox
aborted?: boolean;
}

type TileError = Error & {
status: number;
tileZXYKey: string; // format zoom/x/y
};

interface Tile {
mbKey: string;
mbSourceId: string;
Expand All @@ -23,9 +30,16 @@ interface Tile {

export class TileStatusTracker {
private _tileCache: Tile[];
private _tileErrorCache: Record<string, TileError[]>;
private _prevCenterTileKey?: string;
private readonly _mbMap: MapboxMap;
private readonly _updateTileStatus: (layer: ILayer, areTilesLoaded: boolean) => void;
private readonly _updateTileStatus: (
layer: ILayer,
areTilesLoaded: boolean,
errorMessage?: string
) => void;
private readonly _getCurrentLayerList: () => ILayer[];

private readonly _onSourceDataLoading = (e: MapSourceDataEvent) => {
if (
e.sourceId &&
Expand All @@ -51,16 +65,29 @@ export class TileStatusTracker {
}
};

private readonly _onError = (e: MapSourceDataEvent) => {
private readonly _onError = (e: MapSourceDataEvent & { error: Error & { status: number } }) => {
if (
e.sourceId &&
e.sourceId !== SPATIAL_FILTERS_LAYER_ID &&
e.tile &&
(e.source.type === 'vector' || e.source.type === 'raster')
) {
const targetLayer = this._getCurrentLayerList().find((layer) => {
return layer.ownsMbSourceId(e.sourceId);
});
const layerId = targetLayer ? targetLayer.getId() : undefined;
if (layerId) {
const layerErrors = this._tileErrorCache[layerId] ? this._tileErrorCache[layerId] : [];
layerErrors.push({
...e.error,
tileZXYKey: `${e.tile.tileID.canonical.z}/${e.tile.tileID.canonical.x}/${e.tile.tileID.canonical.y}`,
} as TileError);
this._tileErrorCache[layerId] = layerErrors;
}
this._removeTileFromCache(e.sourceId, e.tile.tileID.key as unknown as string);
}
};

private readonly _onSourceData = (e: MapSourceDataEvent) => {
if (
e.sourceId &&
Expand All @@ -73,23 +100,43 @@ export class TileStatusTracker {
}
};

/*
* Clear errors when center tile changes.
* Tracking center tile provides the cleanest way to know when a new data fetching cycle is beginning
*/
private readonly _onMove = () => {
const center = this._mbMap.getCenter();
// Maplibre rounds zoom when 'source.roundZoom' is true and floors zoom when 'source.roundZoom' is false
// 'source.roundZoom' is true for raster and video layers
// 'source.roundZoom' is false for vector layers
// Always floor zoom to keep logic as simple as possible and not have to track center tile by source.
// We are mainly concerned with showing errors from Elasticsearch vector tile requests (which are vector sources)
const centerTileKey = getTileKey(center.lat, center.lng, Math.floor(this._mbMap.getZoom()));
if (this._prevCenterTileKey !== centerTileKey) {
this._prevCenterTileKey = centerTileKey;
this._tileErrorCache = {};
}
};

constructor({
mbMap,
updateTileStatus,
getCurrentLayerList,
}: {
mbMap: MapboxMap;
updateTileStatus: (layer: ILayer, areTilesLoaded: boolean) => void;
updateTileStatus: (layer: ILayer, areTilesLoaded: boolean, errorMessage?: string) => void;
getCurrentLayerList: () => ILayer[];
}) {
this._tileCache = [];
this._tileErrorCache = {};
this._updateTileStatus = updateTileStatus;
this._getCurrentLayerList = getCurrentLayerList;

this._mbMap = mbMap;
this._mbMap.on('sourcedataloading', this._onSourceDataLoading);
this._mbMap.on('error', this._onError);
this._mbMap.on('sourcedata', this._onSourceData);
this._mbMap.on('move', this._onMove);
}

_updateTileStatusForAllLayers = _.debounce(() => {
Expand All @@ -107,7 +154,31 @@ export class TileStatusTracker {
break;
}
}
this._updateTileStatus(layer, !atLeastOnePendingTile);
const tileErrorMessages = this._tileErrorCache[layer.getId()]
? this._tileErrorCache[layer.getId()].map((tileError) => {
return i18n.translate('xpack.maps.tileStatusTracker.tileErrorMsg', {
defaultMessage: `tile '{tileZXYKey}' failed to load: '{status} {message}'`,
values: {
tileZXYKey: tileError.tileZXYKey,
status: tileError.status,
message: tileError.message,
},
});
})
: [];
this._updateTileStatus(
layer,
!atLeastOnePendingTile,
tileErrorMessages.length
? i18n.translate('xpack.maps.tileStatusTracker.layerErrorMsg', {
defaultMessage: `Unable to load {count} tiles: {tileErrors}`,
values: {
count: tileErrorMessages.length,
tileErrors: tileErrorMessages.join(', '),
},
})
: undefined
);
}
}, 100);

Expand All @@ -126,6 +197,7 @@ export class TileStatusTracker {
this._mbMap.off('error', this._onError);
this._mbMap.off('sourcedata', this._onSourceData);
this._mbMap.off('sourcedataloading', this._onSourceDataLoading);
this._mbMap.off('move', this._onMove);
this._tileCache.length = 0;
}
}
16 changes: 9 additions & 7 deletions x-pack/plugins/maps/server/mvt/get_grid_tile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export async function getEsGridTile({
renderAs: RENDER_AS;
gridPrecision: number;
abortController: AbortController;
}): Promise<{ stream: Stream | null; headers?: IncomingHttpHeaders }> {
}): Promise<{ stream: Stream | null; headers: IncomingHttpHeaders; statusCode: number }> {
try {
const path = `/${encodeURIComponent(index)}/_mvt/${geometryFieldName}/${z}/${x}/${y}`;
const body = {
Expand Down Expand Up @@ -81,13 +81,15 @@ export async function getEsGridTile({
}
);

return { stream: tile.body as Stream, headers: tile.headers };
return { stream: tile.body as Stream, headers: tile.headers, statusCode: tile.statusCode };
} catch (e) {
if (!isAbortError(e)) {
// These are often circuit breaking exceptions
// Should return a tile with some error message
logger.warn(`Cannot generate ES-grid-tile for ${z}/${x}/${y}: ${e.message}`);
if (isAbortError(e)) {
return { stream: null, headers: {}, statusCode: 200 };
}
return { stream: null };

// These are often circuit breaking exceptions
// Should return a tile with some error message
logger.warn(`Cannot generate ES-grid-tile for ${z}/${x}/${y}: ${e.message}`);
return { stream: null, headers: {}, statusCode: 500 };
}
}
16 changes: 9 additions & 7 deletions x-pack/plugins/maps/server/mvt/get_tile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export async function getEsTile({
logger: Logger;
requestBody: any;
abortController: AbortController;
}): Promise<{ stream: Stream | null; headers?: IncomingHttpHeaders }> {
}): Promise<{ stream: Stream | null; headers: IncomingHttpHeaders; statusCode: number }> {
try {
const path = `/${encodeURIComponent(index)}/_mvt/${geometryFieldName}/${z}/${x}/${y}`;

Expand Down Expand Up @@ -81,13 +81,15 @@ export async function getEsTile({
}
);

return { stream: tile.body as Stream, headers: tile.headers };
return { stream: tile.body as Stream, headers: tile.headers, statusCode: tile.statusCode };
} catch (e) {
if (!isAbortError(e)) {
// These are often circuit breaking exceptions
// Should return a tile with some error message
logger.warn(`Cannot generate ES-grid-tile for ${z}/${x}/${y}: ${e.message}`);
if (isAbortError(e)) {
return { stream: null, headers: {}, statusCode: 200 };
}
return { stream: null };

// These are often circuit breaking exceptions
// Should return a tile with some error message
logger.warn(`Cannot generate ES-grid-tile for ${z}/${x}/${y}: ${e.message}`);
return { stream: null, headers: {}, statusCode: 500 };
}
}
Loading

0 comments on commit cd71eac

Please sign in to comment.