From 02c6da18cc106ccc836c0544d39b65fc505f1c74 Mon Sep 17 00:00:00 2001 From: maliming Date: Tue, 16 Jun 2026 14:27:06 +0800 Subject: [PATCH] Auto-set Accept header and unwrap ABP JSON error envelopes in RestService Refs abpframework/abp#23732 --- .../core/src/lib/services/rest.service.ts | 65 +++- .../core/src/lib/tests/rest.service.spec.ts | 291 ++++++++++++++++++ 2 files changed, 354 insertions(+), 2 deletions(-) diff --git a/npm/ng-packs/packages/core/src/lib/services/rest.service.ts b/npm/ng-packs/packages/core/src/lib/services/rest.service.ts index a2d45e2f12..65a98d15f1 100644 --- a/npm/ng-packs/packages/core/src/lib/services/rest.service.ts +++ b/npm/ng-packs/packages/core/src/lib/services/rest.service.ts @@ -1,4 +1,4 @@ -import { HttpClient, HttpParameterCodec, HttpParams, HttpRequest } from '@angular/common/http'; +import { HttpClient, HttpHeaders, HttpParameterCodec, HttpParams, HttpRequest } from '@angular/common/http'; import { Injectable, inject } from '@angular/core'; import { Observable, throwError } from 'rxjs'; import { catchError } from 'rxjs/operators'; @@ -39,7 +39,10 @@ export class RestService { api = api || this.getApiFromStore(config.apiName); const { method, params, ...options } = request; const { observe = Rest.Observe.Body, skipHandleError, responseType = Rest.ResponseType.JSON } = config; + const effectiveResponseType = + ((request as Rest.Request).responseType as Rest.ResponseType | undefined) ?? responseType; const url = this.removeDuplicateSlashes(api + request.url); + const headers = this.ensureAcceptHeader(options.headers, effectiveResponseType); const httpClient: HttpClient = this.getHttpClient(config.skipAddingHeader); return httpClient @@ -50,13 +53,71 @@ export class RestService { params: this.getParams(params, config.httpParamEncoder), }), ...options, + ...(headers && { headers }), } as any) - .pipe(catchError(err => (skipHandleError ? throwError(() => err) : this.handleError(err)))); + .pipe( + catchError(err => { + if (!skipHandleError) { + this.tryUnwrapJsonErrorBody(err, effectiveResponseType); + } + return skipHandleError ? throwError(() => err) : this.handleError(err); + }), + ); } private getHttpClient(isExternal: boolean) { return isExternal ? this.externalHttp : this.http; } + private ensureAcceptHeader( + headers: Rest.Request['headers'], + responseType: Rest.ResponseType, + ): Rest.Request['headers'] | undefined { + const accept = this.getAcceptForResponseType(responseType); + if (!accept) return headers; + if (this.hasAcceptHeader(headers)) return headers; + if (headers instanceof HttpHeaders) { + return headers.set('Accept', accept); + } + return { ...(headers || {}), Accept: accept }; + } + + private hasAcceptHeader(headers: Rest.Request['headers']): boolean { + if (!headers) return false; + if (headers instanceof HttpHeaders) return headers.has('Accept'); + return Object.keys(headers).some(key => key.toLowerCase() === 'accept'); + } + + private getAcceptForResponseType(responseType: Rest.ResponseType): string | null { + switch (responseType) { + case Rest.ResponseType.Blob: + case Rest.ResponseType.ArrayBuffer: + return 'application/octet-stream'; + default: + return null; + } + } + + private tryUnwrapJsonErrorBody(err: any, responseType: Rest.ResponseType): void { + if (responseType === Rest.ResponseType.JSON || !err) return; + if (typeof err.error !== 'string' || err.error.length === 0) return; + let parsed: any; + try { + parsed = JSON.parse(err.error); + } catch { + return; + } + if (this.looksLikeAbpErrorEnvelope(parsed)) { + err.error = parsed; + } + } + + private looksLikeAbpErrorEnvelope(parsed: any): boolean { + if (!parsed || typeof parsed !== 'object' || Array.isArray(parsed)) return false; + const inner = parsed.error; + if (!inner || typeof inner !== 'object') return false; + return typeof inner.code === 'string' || typeof inner.message === 'string'; + } + private getParams(params: Rest.Params, encoder?: HttpParameterCodec): HttpParams { const filteredParams = Object.entries(params).reduce((acc, [key, value]) => { if (isUndefinedOrEmptyString(value)) return acc; diff --git a/npm/ng-packs/packages/core/src/lib/tests/rest.service.spec.ts b/npm/ng-packs/packages/core/src/lib/tests/rest.service.spec.ts index 9ac795c24f..cf967a1d47 100644 --- a/npm/ng-packs/packages/core/src/lib/tests/rest.service.spec.ts +++ b/npm/ng-packs/packages/core/src/lib/tests/rest.service.spec.ts @@ -1,3 +1,4 @@ +import { HttpErrorResponse, HttpHeaders } from '@angular/common/http'; import { createHttpFactory, HttpMethod, SpectatorHttp, SpyObject } from '@ngneat/spectator/vitest'; import { OAuthService } from 'angular-oauth2-oidc'; import { of, throwError } from 'rxjs'; @@ -130,6 +131,296 @@ describe('HttpClient testing', () => { spectator.flushAll([req], [throwError('Testing error')]); }); + test('should set Accept: application/octet-stream when config.responseType is blob', () => { + spectator.service + .request({ method: HttpMethod.GET, url: '/file' }, { responseType: Rest.ResponseType.Blob }) + .subscribe(); + const req = spectator.expectOne(api + '/file', HttpMethod.GET); + expect(req.request.headers.get('Accept')).toEqual('application/octet-stream'); + expect(req.request.responseType).toEqual('blob'); + }); + + test('should set Accept based on request-level responseType (generator path)', () => { + spectator.service + .request({ method: HttpMethod.GET, url: '/file', responseType: 'blob' }) + .subscribe(); + const req = spectator.expectOne(api + '/file', HttpMethod.GET); + expect(req.request.headers.get('Accept')).toEqual('application/octet-stream'); + expect(req.request.responseType).toEqual('blob'); + }); + + test('should set Accept: application/octet-stream when responseType is arraybuffer', () => { + spectator.service + .request( + { method: HttpMethod.GET, url: '/binary' }, + { responseType: Rest.ResponseType.ArrayBuffer }, + ) + .subscribe(); + const req = spectator.expectOne(api + '/binary', HttpMethod.GET); + expect(req.request.headers.get('Accept')).toEqual('application/octet-stream'); + }); + + test('should NOT add Accept for text responseType (left to schematic / caller)', () => { + spectator.service + .request({ method: HttpMethod.GET, url: '/text' }, { responseType: Rest.ResponseType.Text }) + .subscribe(); + const req = spectator.expectOne(api + '/text', HttpMethod.GET); + expect(req.request.headers.has('Accept')).toBe(false); + }); + + test('should not override caller-supplied Accept header (plain object)', () => { + spectator.service + .request( + { method: HttpMethod.GET, url: '/file', headers: { Accept: 'image/png' } }, + { responseType: Rest.ResponseType.Blob }, + ) + .subscribe(); + const req = spectator.expectOne(api + '/file', HttpMethod.GET); + expect(req.request.headers.get('Accept')).toEqual('image/png'); + }); + + test('should not override caller-supplied Accept header (HttpHeaders)', () => { + spectator.service + .request( + { + method: HttpMethod.GET, + url: '/file', + headers: new HttpHeaders({ Accept: 'image/jpeg' }), + }, + { responseType: Rest.ResponseType.Blob }, + ) + .subscribe(); + const req = spectator.expectOne(api + '/file', HttpMethod.GET); + expect(req.request.headers.get('Accept')).toEqual('image/jpeg'); + }); + + test('should preserve caller-supplied non-Accept headers and add Accept', () => { + spectator.service + .request( + { method: HttpMethod.GET, url: '/file', headers: { 'X-Custom': '1' } }, + { responseType: Rest.ResponseType.Blob }, + ) + .subscribe(); + const req = spectator.expectOne(api + '/file', HttpMethod.GET); + expect(req.request.headers.get('Accept')).toEqual('application/octet-stream'); + expect(req.request.headers.get('X-Custom')).toEqual('1'); + }); + + test('should not add Accept header for default JSON responseType', () => { + spectator.service.request({ method: HttpMethod.GET, url: '/json' }).subscribe(); + const req = spectator.expectOne(api + '/json', HttpMethod.GET); + expect(req.request.headers.has('Accept')).toBe(false); + }); + + test('should NOT unwrap error body when skipHandleError is true', async () => { + const spy = vi.spyOn(httpErrorReporter, 'reportError'); + + const completion = new Promise((resolve, reject) => { + spectator.service + .request( + { method: HttpMethod.GET, url: '/text' }, + { responseType: Rest.ResponseType.Text, skipHandleError: true }, + ) + .pipe( + catchError(err => { + try { + expect(spy).toHaveBeenCalledTimes(0); + expect(typeof err.error).toBe('string'); + expect(err.error).toBe('{"error":{"code":"X","message":"y"}}'); + resolve(); + } catch (e) { + reject(e); + } + return of(null); + }), + ) + .subscribe(); + }); + + const req = spectator.expectOne(api + '/text', HttpMethod.GET); + req.flush('{"error":{"code":"X","message":"y"}}', { + status: 500, + statusText: 'Internal Server Error', + }); + + await completion; + }); + + test('should unwrap ABP validationErrors envelope in text mode', async () => { + const spy = vi.spyOn(httpErrorReporter, 'reportError'); + + const completion = new Promise((resolve, reject) => { + spectator.service + .request({ method: HttpMethod.GET, url: '/text' }, { responseType: Rest.ResponseType.Text }) + .pipe( + catchError(() => { + try { + const errArg: any = spy.mock.calls[0][0]; + expect(typeof errArg.error).toBe('object'); + expect(errArg.error.error.message).toBe('Validation failed'); + expect(errArg.error.error.validationErrors).toHaveLength(1); + resolve(); + } catch (e) { + reject(e); + } + return of(null); + }), + ) + .subscribe(); + }); + + const req = spectator.expectOne(api + '/text', HttpMethod.GET); + req.flush( + '{"error":{"message":"Validation failed","validationErrors":[{"message":"Required","members":["Name"]}]}}', + { status: 400, statusText: 'Bad Request' }, + ); + + await completion; + }); + + test('should leave non-ABP-envelope JSON body alone in text mode', async () => { + const completion = new Promise((resolve, reject) => { + spectator.service + .request({ method: HttpMethod.GET, url: '/text' }, { responseType: Rest.ResponseType.Text }) + .pipe( + catchError(err => { + try { + // JSON parses fine but no error.code/error.message → keep raw string + expect(typeof err.error).toBe('string'); + expect(err.error).toBe('{"foo":"bar"}'); + resolve(); + } catch (e) { + reject(e); + } + return of(null); + }), + ) + .subscribe(); + }); + + const req = spectator.expectOne(api + '/text', HttpMethod.GET); + req.flush('{"foo":"bar"}', { status: 500, statusText: 'err' }); + + await completion; + }); + + test('should unwrap JSON-encoded error body in text mode for HttpErrorReporter', async () => { + const spy = vi.spyOn(httpErrorReporter, 'reportError'); + + const completion = new Promise((resolve, reject) => { + spectator.service + .request( + { method: HttpMethod.GET, url: '/text' }, + { responseType: Rest.ResponseType.Text }, + ) + .pipe( + catchError(() => { + try { + expect(spy).toHaveBeenCalledTimes(1); + const errArg: any = spy.mock.calls[0][0]; + expect(errArg.error).toEqual({ + error: { code: 'AbpAuthorization.001', message: 'forbidden' }, + }); + resolve(); + } catch (e) { + reject(e); + } + return of(null); + }), + ) + .subscribe(); + }); + + const req = spectator.expectOne(api + '/text', HttpMethod.GET); + req.flush('{"error":{"code":"AbpAuthorization.001","message":"forbidden"}}', { + status: 403, + statusText: 'Forbidden', + }); + + await completion; + }); + + test('should leave non-JSON error body alone in text mode', async () => { + const spy = vi.spyOn(httpErrorReporter, 'reportError'); + + const completion = new Promise((resolve, reject) => { + spectator.service + .request( + { method: HttpMethod.GET, url: '/text' }, + { responseType: Rest.ResponseType.Text }, + ) + .pipe( + catchError(() => { + try { + const errArg: any = spy.mock.calls[0][0]; + expect(errArg.error).toBe('plain error text'); + resolve(); + } catch (e) { + reject(e); + } + return of(null); + }), + ) + .subscribe(); + }); + + const req = spectator.expectOne(api + '/text', HttpMethod.GET); + req.flush('plain error text', { status: 500, statusText: 'Internal Server Error' }); + + await completion; + }); + + test('should send Accept header emitted by schematic verbatim (json scenario)', () => { + spectator.service + .request({ + method: HttpMethod.GET, + url: '/status', + headers: { Accept: 'application/json' }, + }) + .subscribe(); + const req = spectator.expectOne(api + '/status', HttpMethod.GET); + expect(req.request.headers.get('Accept')).toEqual('application/json'); + }); + + test('should send Accept header emitted by schematic verbatim (text scenario)', () => { + spectator.service + .request({ method: HttpMethod.GET, url: '/csv', headers: { Accept: 'text/plain' } }) + .subscribe(); + const req = spectator.expectOne(api + '/csv', HttpMethod.GET); + expect(req.request.headers.get('Accept')).toEqual('text/plain'); + }); + + test('should leave JSON error body alone in json mode (no double-parse)', async () => { + const spy = vi.spyOn(httpErrorReporter, 'reportError'); + + const completion = new Promise((resolve, reject) => { + spectator.service + .request({ method: HttpMethod.GET, url: '/json' }) + .pipe( + catchError(() => { + try { + const errArg: any = spy.mock.calls[0][0]; + // Angular HttpClient already parsed JSON in json mode → err.error is object + expect(errArg.error).toEqual({ error: { code: 'X', message: 'y' } }); + resolve(); + } catch (e) { + reject(e); + } + return of(null); + }), + ) + .subscribe(); + }); + + const req = spectator.expectOne(api + '/json', HttpMethod.GET); + req.flush( + { error: { code: 'X', message: 'y' } }, + { status: 403, statusText: 'Forbidden' }, + ); + + await completion; + }); + test('should remove the duplicate slashes', () => { spectator.service .request({ method: HttpMethod.GET, url: '//test', params: { id: 1 } })