Skip to content

Commit

Permalink
fix(data-service): fix wrong protocol & improve error handling
Browse files Browse the repository at this point in the history
  • Loading branch information
@jahow
jahow committed Aug 24, 2023
1 parent db59b64 commit db3fb7c
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 62 deletions.
9 changes: 6 additions & 3 deletions libs/feature/dataviz/src/lib/chart-view/chart-view.component.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ export class ChartViewComponent {
private translateService: TranslateService
) {}

handleError(error: FetchError | Error) {
handleError(error: FetchError | Error | string) {
if (error instanceof FetchError) {
this.error = this.translateService.instant(
`dataset.error.${error.type}`,
Expand All @@ -222,9 +222,12 @@ export class ChartViewComponent {
}
)
console.warn(error.message)
} else {
} else if (error instanceof Error) {
this.error = this.translateService.instant(error.message)
console.warn(error.stack)
console.warn(error.stack || error)
} else {
this.error = this.translateService.instant(error)
console.warn(error)
}
this.loading = false
this.changeDetector.detectChanges()
Expand Down
106 changes: 52 additions & 54 deletions libs/feature/dataviz/src/lib/service/data.service.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { TestBed } from '@angular/core/testing'
import { DataService } from './data.service'
import { openDataset } from '@geonetwork-ui/data-fetcher'
import { PROXY_PATH } from '@geonetwork-ui/util/shared'
import { firstValueFrom, lastValueFrom } from 'rxjs'

const newEndpointCall = jest.fn()

Expand Down Expand Up @@ -163,12 +164,12 @@ describe('DataService', () => {
describe('WFS unreachable (CORS)', () => {
it('throws a relevant error', async () => {
try {
await service
.getDownloadLinksFromWfs({
await lastValueFrom(
service.getDownloadLinksFromWfs({
...link,
url: new URL('http://error.cors/wfs'),
})
.toPromise()
)
} catch (e) {
expect(e.message).toBe('wfs.unreachable.cors')
}
Expand All @@ -177,12 +178,12 @@ describe('DataService', () => {
describe('WFS unreachable (HTTP error)', () => {
it('throws a relevant error', async () => {
try {
await service
.getDownloadLinksFromWfs({
await lastValueFrom(
service.getDownloadLinksFromWfs({
...link,
url: new URL('http://error.http/wfs'),
})
.toPromise()
)
} catch (e) {
expect(e.message).toBe('wfs.unreachable.http')
}
Expand All @@ -191,12 +192,12 @@ describe('DataService', () => {
describe('WFS unreachable (unknown)', () => {
it('throws a relevant error', async () => {
try {
await service
.getDownloadLinksFromWfs({
await lastValueFrom(
service.getDownloadLinksFromWfs({
...link,
url: new URL('http://error/wfs'),
})
.toPromise()
)
} catch (e) {
expect(e.message).toBe('wfs.unreachable.unknown')
}
Expand All @@ -205,25 +206,25 @@ describe('DataService', () => {
describe('WFS with unknown feature type', () => {
it('throws a relevant error', async () => {
try {
await service
.getDownloadLinksFromWfs({
await lastValueFrom(
service.getDownloadLinksFromWfs({
...link,
name: 'featuretype_missing',
})
.toPromise()
)
} catch (e) {
expect(e.message).toBe('wfs.featuretype.notfound')
}
})
})
describe('WFS with GeoJSON support', () => {
it('returns a list of links', async () => {
const urls = await service
.getDownloadLinksFromWfs({
const urls = await lastValueFrom(
service.getDownloadLinksFromWfs({
...link,
url: new URL('http://local/wfs'),
})
.toPromise()
)
expect(urls).toEqual([
{
description: 'Lieu de surveillance (ligne)',
Expand Down Expand Up @@ -270,13 +271,13 @@ describe('DataService', () => {
})
describe('WFS without GeoJSON support', () => {
it('returns a list of links (without geojson)', async () => {
const urls = await service
.getDownloadLinksFromWfs({
const urls = await lastValueFrom(
service.getDownloadLinksFromWfs({
...link,
url: new URL('http://local/wfs'),
name: 'nojson_type',
})
.toPromise()
)
expect(urls).toEqual([
{
description: 'Lieu de surveillance (ligne)',
Expand Down Expand Up @@ -313,13 +314,13 @@ describe('DataService', () => {
})
describe('WFS with only one feature type, no feature type name specified', () => {
it('returns a list of links using the only feature type', async () => {
const urls = await service
.getDownloadLinksFromWfs({
const urls = await lastValueFrom(
service.getDownloadLinksFromWfs({
...link,
url: new URL('http://unique-feature-type/wfs'),
name: '',
})
.toPromise()
)
expect(urls).toEqual([
{
description: 'Lieu de surveillance (ligne)',
Expand Down Expand Up @@ -370,10 +371,9 @@ describe('DataService', () => {
describe('#getGeoJsonDownloadUrlFromWfs', () => {
describe('WFS with GeoJSON support', () => {
it('returns an url', async () => {
const url = await service
.getDownloadUrlsFromWfs('http://local/wfs', 'abcd')
.toPromise()
.then((urls) => urls.geojson)
const url = await lastValueFrom(
service.getDownloadUrlsFromWfs('http://local/wfs', 'abcd')
).then((urls) => urls.geojson)
expect(url).toEqual(
'http://local/wfs?GetFeature&FeatureType=abcd&format=application/json'
)
Expand All @@ -382,20 +382,19 @@ describe('DataService', () => {
describe('WFS without GeoJSON support', () => {
it('returns an observable that errors with a relevant error', async () => {
try {
await service
.getDownloadUrlsFromWfs('http://local/wfs', 'nojsontype')
.toPromise()
await lastValueFrom(
service.getDownloadUrlsFromWfs('http://local/wfs', 'nojsontype')
)
} catch (e) {
expect(e.message).toBe('wfs.geojsongml.notsupported')
}
})
})
describe('WFS with only one feature type, no feature type name specified', () => {
it('returns one valid link using the only feature type', async () => {
const url = await service
.getDownloadUrlsFromWfs('http://unique-feature-type/wfs', '')
.toPromise()
.then((urls) => urls.geojson)
const url = await lastValueFrom(
service.getDownloadUrlsFromWfs('http://unique-feature-type/wfs', '')
).then((urls) => urls.geojson)
expect(url).toEqual(
'http://unique-feature-type/wfs?GetFeature&FeatureType=myOnlyOne&format=application/json'
)
Expand Down Expand Up @@ -450,13 +449,13 @@ describe('DataService', () => {
describe('parse failure', () => {
it('returns an observable that errors with a relevant error', async () => {
try {
await service
.getDataset({
await lastValueFrom(
service.getDataset({
url: new URL('http://error.parse/geojson'),
mimeType: 'application/geo+json',
type: 'download',
})
.toPromise()
)
} catch (e) {
expect(e).toStrictEqual({
info: 'Something went wrong',
Expand All @@ -468,13 +467,13 @@ describe('DataService', () => {
describe('CORS or network error', () => {
it('returns an observable that errors with a relevant error', async () => {
try {
await service
.getDataset({
await lastValueFrom(
service.getDataset({
url: new URL('http://error.network/xls'),
mimeType: 'application/vnd.ms-excel',
type: 'download',
})
.toPromise()
)
} catch (e) {
expect(e).toStrictEqual({
info: 'Something went wrong',
Expand All @@ -486,13 +485,13 @@ describe('DataService', () => {
describe('HTTP error', () => {
it('returns an observable that errors with a relevant error', async () => {
try {
await service
.getDataset({
await lastValueFrom(
service.getDataset({
url: new URL('http://error.http/csv'),
mimeType: 'text/csv',
type: 'download',
})
.toPromise()
)
} catch (e) {
expect(e).toStrictEqual({
info: 'Something went wrong',
Expand All @@ -505,13 +504,13 @@ describe('DataService', () => {
describe('unknown error', () => {
it('returns an observable that errors with a relevant error', async () => {
try {
await service
.getDataset({
await lastValueFrom(
service.getDataset({
url: new URL('http://error/xls'),
mimeType: 'application/vnd.ms-excel',
type: 'download',
})
.toPromise()
)
} catch (e) {
expect(e).toStrictEqual({
info: 'Something went wrong',
Expand All @@ -532,13 +531,13 @@ describe('DataService', () => {
)
})
it('returns an observable that emits the array of features', async () => {
const result = await service
.getDataset({
const result = await lastValueFrom(
service.getDataset({
url: new URL('http://sample/csv'),
mimeType: 'text/csv',
type: 'download',
})
.toPromise()
)
await expect(result.read()).resolves.toEqual(SAMPLE_GEOJSON.features)
})
})
Expand All @@ -547,13 +546,13 @@ describe('DataService', () => {
describe('#readGeoJsonDataset', () => {
describe('valid file', () => {
it('returns an observable that emits the feature collection', async () => {
const result = await service
.readAsGeoJson({
const result = await lastValueFrom(
service.readAsGeoJson({
url: new URL('http://sample/geojson'),
mimeType: 'application/geo+json',
type: 'download',
})
.toPromise()
)
expect(result).toEqual(SAMPLE_GEOJSON)
})
})
Expand Down Expand Up @@ -581,10 +580,9 @@ describe('DataService', () => {
)
})
it('returns a proxied url', async () => {
const url = await service
.getDownloadUrlsFromWfs('http://local/wfs', 'abcd')
.toPromise()
.then((urls) => urls.geojson)
const url = await lastValueFrom(
service.getDownloadUrlsFromWfs('http://local/wfs', 'abcd')
).then((urls) => urls.geojson)
expect(url).toEqual(
'http://proxy.local/?url=http%3A%2F%2Flocal%2Fwfs?GetFeature&FeatureType=abcd&format=application/json'
)
Expand Down
4 changes: 2 additions & 2 deletions libs/feature/dataviz/src/lib/service/data.service.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ export class DataService {

getDataset(link: DatasetDistribution): Observable<BaseReader> {
const linkUrl = this.proxy.getProxiedUrl(link.url.toString())
if (link.type === 'service' && link.accessServiceProtocol === 'wms') {
if (link.type === 'service' && link.accessServiceProtocol === 'wfs') {
return this.getDownloadUrlsFromWfs(linkUrl, link.name).pipe(
switchMap((urls) => {
if (urls.geojson) return openDataset(urls.geojson, 'geojson')
Expand Down Expand Up @@ -200,6 +200,6 @@ export class DataService {
const url = this.getDownloadUrlFromEsriRest(linkUrl, 'geojson')
return from(openDataset(url, 'geojson')).pipe()
}
return throwError('protocol not supported')
return throwError(() => 'protocol not supported')
}
}
9 changes: 6 additions & 3 deletions libs/feature/dataviz/src/lib/table-view/table-view.component.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export class TableViewComponent {
console.log(event)
}

handleError(error: FetchError | Error) {
handleError(error: FetchError | Error | string) {
if (error instanceof FetchError) {
this.error = this.translateService.instant(
`dataset.error.${error.type}`,
Expand All @@ -78,9 +78,12 @@ export class TableViewComponent {
}
)
console.warn(error.message)
} else {
} else if (error instanceof Error) {
this.error = this.translateService.instant(error.message)
console.warn(error.stack)
console.warn(error.stack || error)
} else {
this.error = this.translateService.instant(error)
console.warn(error)
}
this.loading = false
}
Expand Down

0 comments on commit db3fb7c

Please sign in to comment.