Auto-set Accept header and unwrap ABP JSON error envelopes in RestService

Refs abpframework/abp#23732

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<T>).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<unknown>['headers'],
+    responseType: Rest.ResponseType,
+  ): Rest.Request<unknown>['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<unknown>['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;

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<void>((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<void>((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<void>((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<void>((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<void>((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<void>((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 } })