Parse Server should return consistent error messages

[danielsanfr]

New Issue Checklist

• I am not disclosing a vulnerability.
• I am not just asking a question.
• I have searched through existing issues.
• I can reproduce the issue with the latest version of Parse Server.

Issue Description

The Parse Server should return consistent error messages. However, it is returning a message field when an internal error occurs, see:

parse-server/src/middlewares.js

Lines 375 to 378 in d8dc524

	res.json({
	code: Parse.Error.INTERNAL_SERVER_ERROR,
	message: 'Internal server error.',
	});

However, if you look at the other returns in:

parse-server/src/middlewares.js

Line 364 in d8dc524

res.json({ code: err.code, error: err.message });

and

parse-server/src/middlewares.js

Line 368 in d8dc524

res.json({ error: err.message });

Steps to reproduce

Sorry, but I don't know how to reproduce it consistently.

Actual Outcome

{ 
   "code": 1, 
   "message": "Internal server error." 
}

Expected Outcome

{ 
   "code": 1, 
   "error": "Internal server error."
}

Failing Test Case / Pull Request

• 🤩 I submitted a PR with a fix and a test case.
• 🧐 I submitted a PR with a failing test case.

Environment

Server

• Parse Server version: 4.5.0
• Operating system: Linux
• Local or remote host (AWS, Azure, Google Cloud, Heroku, Digital Ocean, etc): Both

Database

• System (MongoDB or Postgres): Both
• Database version: Any
• Local or remote host (MongoDB Atlas, mLab, AWS, Azure, Google Cloud, etc): Both

Client

• SDK (iOS, Android, JavaScript, PHP, Unity, etc): Anyone
• SDK version: Anyone

Logs

Not applicable

[davimacedo]

I think it makes sense and it would be good to be consistent on that. Not sure if we should go with { code, message} or { code, error } though. We need also to understand the impact on the different sdks. @mtrezza @dplewis thoughts?

[mtrezza]

Agree the key should be consistent. We also have a related issue regarding consistency of the error message itself.

Regarding key name, I would suggest message rather than error to make it semantically more readable. "An error has a message and a code" is more readable than "an error has an error and a code". In code it also makes variable naming more natural:

let error: PFError
let message = error.message

rather than

let error: PFError
let whatDoINameThis = error.error

[danielsanfr]

I even agree with you @mtrezza, but the REST documentation doesn't mention the message field, so my suggestion was to follow the standard that is adopted now. If we don't keep the error field, it will definitely break the SDKs and whoever is using REST.

[mtrezza]

Well, mere semantic aesthetics don't justify breaking changes. So if error breaks less, then let's go with that.

[cbaker6]

The SDK’s actually use code and message, they don’t use error, which will break most of the SDKs which use REST:

• JS SDK:

  constructor(code, message) {
    super(message);
    this.code = code;
    Object.defineProperty(this, 'message', {
      enumerable: true,
      value: message,
    });
  }

• Obj-c SDK

+ (NSError *)errorWithCode:(NSInteger)code message:(NSString *)message;
+ (NSError *)errorWithCode:(NSInteger)code message:(NSString *)message shouldLog:(BOOL)shouldLog;

• Android SDK

    /**
     * Construct a new ParseException with an external cause.
     *
     * @param message A message describing the error in more detail.
     * @param cause   The cause of the error.
     */
    public ParseException(int theCode, String message, Throwable cause) {
        super(message, cause);
        code = theCode;
    }

• Swift SDK

public struct ParseError: ParseType, Decodable, Swift.Error {
    /// The value representing the error from the Parse Server.
    public let code: Code
    /// The text representing the error from the Parse Server.
    public let message: String
    /// An error value representing a custom error from the Parse Server.
    public let otherCode: Int?
    init(code: Code, message: String) {
        self.code = code
        self.message = message
        self.otherCode = nil
    }

[danielsanfr]

@cbaker6 on the public API the name of the field is message, but the JSON send from the Parse Server and read from the clients SDKs and the REST is error,

For example, if you go to the REST guide and search for "message" you will not found any result, but if search for "error" you will found 4 results.

Alternatively you can go straight to the Response Format section in the REST guide, where you will find exactly the information I said.

[cbaker6]

@cbaker6 on the public API the name of the field is message, but the JSON send from the Parse Server and read from the clients SDKs and the REST is error,

For example, if you go to the REST guide and search for "message" you will not found any result, but if search for "error" you will found 4 results.

Alternatively you can go straight to the Response Format section in the REST guide, where you will find exactly the information I said.

This isn't true, the Swift SDK decodes directly from JSON with a strict decoder:

    public init(from decoder: Decoder) throws {
        let values = try decoder.container(keyedBy: CodingKeys.self)
        do {
            code = try values.decode(Code.self, forKey: .code)
            otherCode = nil
        } catch {
            code = .other
            otherCode = try values.decode(Int.self, forKey: .code)
        }
        message = try values.decode(String.self, forKey: .message)
    }

In addition, you can check the JS SDK ParseError encoding test:

it('has a proper json representation', () => {
    const error = new ParseError(123, 'some error message');
    expect(JSON.parse(JSON.stringify(error))).toEqual({
      message: 'some error message',
      code: 123,
    });
  });

console log output from the server:

parse_1      | error: Error handling request: ParseError: An appName, publicServerURL, and emailAdapter are required for password reset and email verification functionality.
parse_1      |     at UsersRouter._throwOnBadEmailConfig (/parse-server/lib/Routers/UsersRouter.js:315:15)
parse_1      |     at UsersRouter.handleVerificationEmailRequest (/parse-server/lib/Routers/UsersRouter.js:356:10)
parse_1      |     at /parse-server/lib/Routers/UsersRouter.js:427:19
parse_1      |     at /parse-server/lib/PromiseRouter.js:175:7
parse_1      |     at Layer.handle [as handle_request] (/parse-server/node_modules/express/lib/router/layer.js:95:5)
parse_1      |     at next (/parse-server/node_modules/express/lib/router/route.js:137:13)
parse_1      |     at Route.dispatch (/parse-server/node_modules/express/lib/router/route.js:112:3)
parse_1      |     at Layer.handle [as handle_request] (/parse-server/node_modules/express/lib/router/layer.js:95:5)
parse_1      |     at /parse-server/node_modules/express/lib/router/index.js:281:22
parse_1      |     at Function.process_params (/parse-server/node_modules/express/lib/router/index.js:335:12) {
parse_1      |   code: 1
parse_1      | } {"error":{"message":"An appName, publicServerURL, and emailAdapter are required for password reset and email verification functionality.","code":1}}

[danielsanfr]

@cbaker6 see this enum:

https://github.com/parse-community/Parse-Swift/blob/457ef7ce978640e8990cc844830599bbfe02588e/Sources/ParseSwift/Types/ParseError.swift#L36-L39

I didn't search all the Android SDK code, but I quickly found it here:

https://github.com/parse-community/Parse-SDK-Android/blob/ea2a6a7d7b761a568dfb261cd0694699f3c5a978/parse/src/main/java/com/parse/ParseRESTObjectBatchCommand.java#L122-L133

[cbaker6]

@cbaker6 see this enum:

https://github.com/parse-community/Parse-Swift/blob/457ef7ce978640e8990cc844830599bbfe02588e/Sources/ParseSwift/Types/ParseError.swift#L36-L39

This would make it decode based on error, meaning that you are right

[danielsanfr]

In addition, you can check the JS SDK ParseError encoding test:

It doesn't test the function in question (handleParseErrors), it test if the Parse.Error "has a proper json representation".

And I can't find a test for the handleParseErrors function easily 😔.

[cbaker6]

@danielsanfr sorry for the mistake.

It seems your PR is valid as there is confusion. It seems many of the SDKs decode the error key properly and then use the message key instead. I'm guessing someone noticed the issues with error at some point and made the change across the board. I would guess the reason for message is because of what @mtrezza mentioned which I agree with: #7444 (comment)

[guilhermetod]

Hey guys, I'd like to add another problem we're facing that I think it's related to this and maybe could go on the same PR. In the org I work for, we have a repo with cloud functions that had to be deployed without using the Parse.Cloud.Define.

The problem is that the response is sent as JSON, so when it's parsed it looses its prototype, so the error is not a Parse.Errorobject anymore and it doesn't fall into this condition. This wouldn't be much of a huge problem, but because it doesn't fall into this condition, we loose the error code and instead get a generic 141 error. So when we send a response like

{ message: 'This is a custom error', code: 999 }

We receive

{ message: 'This is a custom error', code: 141 }

We could change this line to avoid error code modification, which from my point of view is an unexpected behaviour (even though I recognize we might not be using the cloud functions in the ideal way):

const code = message.code ?? defaultOpts.code ?? Parse.Error.SCRIPT_FAILED;

[mtrezza]

@guilhermetod Thanks for your comment. This is an interesting topic, but it seems unrelated to this issue. Would you mind opening a separate issue to discuss this? There may be an existing issue or even a PR, because this topic pops up frequently, see:

• https://community.parseplatform.org/t/how-to-return-403-when-user-is-not-set-in-cloud-function-request/900/5
• https://community.parseplatform.org/t/server-wide-custom-error-messages/255