OneSignal / onesignal-node-api

OneSignal Node Client
Other
28 stars 12 forks source link

[Bug]: createNotification returns HTTP-Code: 400 Message: Bad Request Body: {"errors":[{}]} #77

Closed ArpadGBondor closed 2 months ago

ArpadGBondor commented 5 months ago

What happened?

I'm trying to implement a function that we can use to send notifications using OneSignal. The error I'm getting makes no sense to me.

Steps to reproduce?

static async createNotification(title: string, message: string, userEmail: string, languageCode: string = 'en') {
    const notification = new OneSignal.Notification();

    notification.name = 'Admin notification';
    notification.contents = {
      [languageCode]: message,
    };
    notification.headings = {
      [languageCode]: title,
    };
    notification.filters = [
      {
        field: 'tag',
        key: 'email', // we agreed to use email for the tags
        relation: '=',
        value: userEmail,
      },
    ];

    try {
      return await client.createNotification(notification);
    } catch (error: any) {
      console.error(error.message);
    }

    return 'ERROR';
  }

What did you expect to happen?

Either create the notification, or give me an error message that makes sense.

Relevant log output

No response

Code of Conduct

ArpadGBondor commented 5 months ago

I figured it out.

So, the documentation says to configure your app like this:

const configuration = OneSignal.createConfiguration({
    userKey: '<YOUR_USER_KEY_TOKEN>',
    appKey: '<YOUR_APP_KEY_TOKEN>',
});

const client = new OneSignal.DefaultApi(configuration);

The problem is, that the API wants the API key. Not the USER_KEY_TOKEN and not the APP_KEY_TOKEN.

once I changed the configuration, it started working:

const configuration = OneSignal.createConfiguration({
    appKey: '<API_KEY_TOKEN>',
});

Update:

Setting the API KEY solved one problem, but the create Notification function kept throwing HTTP ERROR 400 with no understandable error descriptions in the body.

I started experimenting with Postman to see what properties needs to be added to the notification object. Here's the version of the function that worked in the end:

  static async createNotification(title: string, message: string, userEmail: string, languageCode: string = 'en') {
    const client = new OneSignal.DefaultApi(configuration);
    const notification = new OneSignal.Notification();

    notification.app_id = <APP ID HERE>;
    notification.url = 'http://localhost:3000';
    notification.target_channel = 'push';
    notification.name = 'Admin notification';
    notification.contents = {
      [languageCode]: message,
    };
    notification.headings = {
      [languageCode]: title,
    };
    notification.filters = [
      {
        field: 'tag',
        key: 'email', // we agreed to use email for the tags
        relation: '=',
        value: userEmail,
      },
    ];

    try {
      return await client.createNotification(notification, configuration);
    } catch (error: any) {
      console.error(error.message);
    }

    return 'ERROR';
  }
wdifruscio commented 3 months ago

brilliant client side SDKs, absolutely esoteric server side sdk implementation lol

selected-pixel-jameson commented 2 months ago

Their documentation is absolutely garbage and has been since their existence. Who is going to setup a legit push notification service that doesn't implement a back-end solution for sending out notifications. WOW. This is unbelievable.

Thank god you've provided this. I can't believe this has been open and unaddressed for 2 months!

Elyytscha commented 2 months ago

I mean someone could argument just stop beeing a react andy 🤣

https://www.instagram.com/reel/CqB253CP42e/

ArpadGBondor commented 2 months ago

I mean someone could argument just stop beeing a react andy 🤣

https://www.instagram.com/reel/CqB253CP42e/

I don't think that your comment is relevant, or constructive. Being able to use the language and data structures has absolutely nothing to do with using badly implemented, badly documented and badly maintained libraries. Overall the guy on the video is not completely wrong. After a week of struggling with OneSignal's tools, I advised my manager to avoid using OneSignal, and implement ourselves what we need. It's already done, and deployed. :)

Elyytscha commented 2 months ago

I mean it was meant as a bad joke, obviously I stumbled upon this Issue because I had the same problem.

iAmWillShepherd commented 2 months ago

@ArpadGBondor, I'm glad you figured this out.

Looks like you got caught by a typo in our docs!

What exists

const configuration = OneSignal.createConfiguration({
    userKey: '<YOUR_USER_KEY_TOKEN>',
    appKey: '<YOUR_APP_KEY_TOKEN>',
});

What it should be

const configuration = OneSignal.createConfiguration({
    userKey: '<YOUR_USER_KEY_TOKEN>',
    appKey: '<YOUR_API_KEY_TOKEN>', // just one letter off 🤦🏽‍♂️
});

This is confusing indeed, and I think the best course of action is to change the name of that key from appKey to apiKey. IMO, appKey makes no sense because we don't have a concept for that anymore. I'll ensure I bring this up to the SDK team, as I've been caught by this before too.

I've gone ahead and updated the docs 🙏🏽

FYI there's a sample project for Node that shows how to correctly set this stuff up and even use some of the functions, albeit to a limited amount.

EskelCz commented 2 months ago

In my case it was missing ENV keys. The error message was fixed when I downgraded to node-onesignal@1.0.0-beta7