Repository: getsentry/sentry-php (php)

Issue:
Project IDs should be strings
### Environment

How do you use Sentry?
Sentry SaaS (sentry.io) or self-hosted/on-premise (which version?)

Which SDK and version?
PHP

### Steps to Reproduce

https://github.com/getsentry/sentry-php/blob/ea704bbe145657a507d73cd8bf6cc017d269fffa/src/Dsn.php#L102

### Expected Result

Project IDs should be cast as strings, not int

### Actual Result

Project IDs are int in from PHP sdks

we need project ID to not be forced to an integer because its plausible we will need it to be a UUID, or we will need it to be a 64 bit int (which breaks in JS, as an example)


Pull request:
Treat the project ID component of the DSN as a `string` rather than an `integer`
Fixes #1292. With this PR, the `Dsn::getProjectId()` method deprecates returning the project ID as an `integer`. In the next major version, we can reintroduce the return type and make it `string`. It looks like version `2.x` was correct, but this behaviour was lost when we introduced the `Dsn` class. I'm unsure whether we want to classify this as a bug or as a feature. In any case, we cannot avoid the code to handle both typehints as changing it would be a breaking change

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2c774f1..d6871c0 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,7 @@
 - Set the event extras by taking the data from the Monolog record's context (#1244)
 - Make the `StacktraceBuilder` class part of the public API and add the `Client::getStacktraceBuilder()` method to build custom stacktraces (#1124)
 - Support handling the server rate-limits when sending events to Sentry (#1291)
+- Treat the project ID component of the DSN as a `string` rather than an `integer` (#1293)
 
 ## 3.3.7 (2022-01-19)
 
diff --git a/src/Dsn.php b/src/Dsn.php
index e96d448..bf54ea4 100644
--- a/src/Dsn.php
+++ b/src/Dsn.php
@@ -38,7 +38,7 @@ final class Dsn implements \Stringable
     private $secretKey;
 
     /**
-     * @var int The ID of the resource to access
+     * @var string The ID of the resource to access
      */
     private $projectId;
 
@@ -53,12 +53,12 @@ final class Dsn implements \Stringable
      * @param string      $scheme    The protocol to be used to access the resource
      * @param string      $host      The host that holds the resource
      * @param int         $port      The port on which the resource is exposed
-     * @param int         $projectId The ID of the resource to access
+     * @param string      $projectId The ID of the resource to access
      * @param string      $path      The specific resource that the web client wants to access
      * @param string      $publicKey The public key to authenticate the SDK
      * @param string|null $secretKey The secret key to authenticate the SDK
      */
-    private function __construct(string $scheme, string $host, int $port, int $projectId, string $path, string $publicKey, ?string $secretKey)
+    private function __construct(string $scheme, string $host, int $port, string $projectId, string $path, string $publicKey, ?string $secretKey)
     {
         $this->scheme = $scheme;
         $this->host = $host;
@@ -98,11 +98,6 @@ public static function createFromString(string $value): self
 
         $segmentPaths = explode('/', $parsedDsn['path']);
         $projectId = array_pop($segmentPaths);
-
-        if ((int) $projectId <= 0) {
-            throw new \InvalidArgumentException('"%s" DSN must contain a valid project ID.');
-        }
-
         $lastSlashPosition = strrpos($parsedDsn['path'], '/');
         $path = $parsedDsn['path'];
 
@@ -114,7 +109,7 @@ public static function createFromString(string $value): self
             $parsedDsn['scheme'],
             $parsedDsn['host'],
             $parsedDsn['port'] ?? ('http' === $parsedDsn['scheme'] ? 80 : 443),
-            (int) $projectId,
+            $projectId,
             $path,
             $parsedDsn['user'],
             $parsedDsn['pass'] ?? null
@@ -155,10 +150,18 @@ public function getPath(): string
 
     /**
      * Gets the ID of the resource to access.
+     *
+     * @return int|string
      */
-    public function getProjectId(): int
+    public function getProjectId(bool $returnAsString = false)
     {
-        return $this->projectId;
+        if ($returnAsString) {
+            return $this->projectId;
+        }
+
+        @trigger_error(sprintf('Calling the method %s() and expecting it to return an integer is deprecated since version 3.4 and will stop working in 4.0.', __METHOD__), \E_USER_DEPRECATED);
+
+        return (int) $this->projectId;
     }
 
     /**


Test patch:
diff --git a/tests/DsnTest.php b/tests/DsnTest.php
index 1681367..6c31e99 100644
--- a/tests/DsnTest.php
+++ b/tests/DsnTest.php
@@ -6,9 +6,12 @@
 
 use PHPUnit\Framework\TestCase;
 use Sentry\Dsn;
+use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
 
 final class DsnTest extends TestCase
 {
+    use ExpectDeprecationTrait;
+
     /**
      * @dataProvider createFromStringDataProvider
      */
@@ -19,7 +22,7 @@ public function testCreateFromString(
         int $expectedPort,
         string $expectedPublicKey,
         ?string $expectedSecretKey,
-        int $expectedProjectId,
+        string $expectedProjectId,
         string $expectedPath
     ): void {
         $dsn = Dsn::createFromString($value);
@@ -29,7 +32,7 @@ public function testCreateFromString(
         $this->assertSame($expectedPort, $dsn->getPort());
         $this->assertSame($expectedPublicKey, $dsn->getPublicKey());
         $this->assertSame($expectedSecretKey, $dsn->getSecretKey());
-        $this->assertSame($expectedProjectId, $dsn->getProjectId());
+        $this->assertSame($expectedProjectId, $dsn->getProjectId(true));
         $this->assertSame($expectedPath, $dsn->getPath());
     }
 
@@ -42,7 +45,7 @@ public function createFromStringDataProvider(): \Generator
             80,
             'public',
             null,
-            1,
+            '1',
             '/sentry',
         ];
 
@@ -53,7 +56,7 @@ public function createFromStringDataProvider(): \Generator
             80,
             'public',
             null,
-            1,
+            '1',
             '',
         ];
 
@@ -64,7 +67,7 @@ public function createFromStringDataProvider(): \Generator
             80,
             'public',
             'secret',
-            1,
+            '1',
             '',
         ];
 
@@ -75,7 +78,7 @@ public function createFromStringDataProvider(): \Generator
             80,
             'public',
             null,
-            1,
+            '1',
             '',
         ];
 
@@ -86,7 +89,7 @@ public function createFromStringDataProvider(): \Generator
             8080,
             'public',
             null,
-            1,
+            '1',
             '',
         ];
 
@@ -97,7 +100,7 @@ public function createFromStringDataProvider(): \Generator
             443,
             'public',
             null,
-            1,
+            '1',
             '',
         ];
 
@@ -108,7 +111,7 @@ public function createFromStringDataProvider(): \Generator
             443,
             'public',
             null,
-            1,
+            '1',
             '',
         ];
 
@@ -119,7 +122,7 @@ public function createFromStringDataProvider(): \Generator
             4343,
             'public',
             null,
-            1,
+            '1',
             '',
         ];
     }
@@ -171,16 +174,6 @@ public function createFromStringThrowsExceptionIfValueIsInvalidDataProvider(): \
             'tcp://public:secret@example.com/1',
             'The scheme of the "tcp://public:secret@example.com/1" DSN must be either "http" or "https".',
         ];
-
-        yield 'invalid project ID (char instead of number)' => [
-            'http://public:secret@example.com/j',
-            'DSN must contain a valid project ID.',
-        ];
-
-        yield 'invalid project ID (negative number)' => [
-            'http://public:secret@example.com/-2',
-            'DSN must contain a valid project ID.',
-        ];
     }
 
     /**
@@ -240,4 +233,16 @@ public function toStringDataProvider(): array
             ['https://public@example.com:4343/sentry/1'],
         ];
     }
+
+    /**
+     * @group legacy
+     */
+    public function testGetProjectIdTriggersDeprecationErrorIfReturningInteger(): void
+    {
+        $dsn = Dsn::createFromString('https://public@example.com/sentry/1');
+
+        $this->expectDeprecation('Calling the method Sentry\\Dsn::getProjectId() and expecting it to return an integer is deprecated since version 3.4 and will stop working in 4.0.');
+
+        $this->assertSame(1, $dsn->getProjectId());
+    }
 }